Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Encrypt the oauth client secrets #446

Merged
merged 3 commits into from
Jul 28, 2023
Merged

Encrypt the oauth client secrets #446

merged 3 commits into from
Jul 28, 2023

Conversation

SwikritiT
Copy link
Contributor

@SwikritiT SwikritiT commented Jul 27, 2023 •
edited
Loading

Description

The server implemented the feature to encrypt the oauth client secret in nextcloud/server#38398
but these changes were not adapted to our app so the OAuth connection wasn't successful from the open project side.

As this feature is only available from the nextcloud

  • >= 25.0.8
  • >= 26.0.4
  • >= 27.0.1

So the necessary check to ensure the version of the server is made.

Also the client secret is displayed only once now, right after it's created.

Related workpackage

OP#49416 - https://community.openproject.org/projects/nextcloud-integration/work_packages/49416

@SwikritiT SwikritiT marked this pull request as ready for review July 28, 2023 08:40
@SwikritiT SwikritiT force-pushed the encrypt-the-oauth-client-secrets branch from 7d4bd5f to 46ef426 Compare July 28, 2023 10:28
@SwikritiT
Encrypt the OAuth2 client secrets
c5eca7f 
check versions of nc

add migration step

update version and extensions

try running qb

disable migration

enable migration

enable migration

enable migration

enable migration

enable migration

remove migration

add more version checks

add more version checks

add migration

add migration

add extension

don't show secrets

update unit tests

fix api test

fix api test

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
@SwikritiT SwikritiT force-pushed the encrypt-the-oauth-client-secrets branch from 46ef426 to c5eca7f Compare July 28, 2023 10:30
@SwikritiT
remove stable22 from api test
61a7eb5 
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
@SwikritiT
Copy link
Contributor Author

The api test realted to the files id endpoint keep failing with stable22 because for some reason activity app is not being downloaded https://github.com/nextcloud/integration_openproject/actions/runs/5690981026/job/15425312895#step:16:157
However, these changes should not affect the working of the files id endpoint.

In this run https://github.com/nextcloud/integration_openproject/actions/runs/5691122946 I removed the stable22 for api tests to be sure that tests are passing for other stable versions. If the CI is green for that one I'll bring back the stable22 and if the CI is red we might need to merge with red CI, as I don't see any other solution to fix the activity app not being enabled for stable 22

SagarGi
SagarGi approved these changes Jul 28, 2023
View reviewed changes
Copy link
Collaborator

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@SwikritiT
bring back stable22 for api test
ff62900 
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
@SwikritiT
Copy link
Contributor Author

This is the run where all the unit test passed https://github.com/nextcloud/integration_openproject/actions/runs/5689688522

This is the run where all the api tests passed https://github.com/nextcloud/integration_openproject/actions/runs/5688758938

The do pass

@github-actions
Copy link

JS Code Coverage

Coverage after merging encrypt-the-oauth-client-secrets into release/2.3 will be
93.82%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   adminSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   bootstrap.js0%0%0%0%1, 1–7
   dashboard.js0%0%0%0%1, 1, 10–19, 2, 20–27, 3–9
   fileActions.js0%0%0%0%1, 1, 10–17, 2–9
   personalSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   projectTab.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–66, 7–9
   utils.js57.45%33.33%50%59.52%10–14, 17–26, 6–9
src/components
   AdminSettings.vue99.23%95.83%82.35%99.86%1, 1, 1
   OAuthConnectButton.vue99.09%80%100%100%1
   PersonalSettings.vue98.88%91.67%83.33%100%1
src/components/admin
   FieldValue.vue93.26%28.57%100%98.77%1, 1, 1, 1, 1, 1
   FormHeading.vue98.98%75%100%100%1
   TextInput.vue98.46%80%87.50%100%1, 1, 1
src/components/icons
   ClippyIcon.vue93.18%50%50%97.50%1, 1
src/components/settings
   CheckBox.vue92.45%80%66.67%97.62%1, 1
   SettingsTitle.vue94.74%50%100%97.14%1, 1
src/components/tab
   EmptyContent.vue99.34%90.91%100%100%1
   SearchInput.vue99.56%83.33%100%100%1
   WorkPackage.vue99.01%33.33%100%99.66%1, 1, 1
src/utils
   workpackageHelper.js97.46%96%100%97.75%17–19
src/views
   Dashboard.vue98.36%50%50%99.66%1, 1, 1
   ProjectsTab.vue99.74%92.31%100%100%23

@github-actions
Copy link

PHP Code Coverage

Coverage after merging encrypt-the-oauth-client-secrets into release/2.3 will be
59.68%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib/AppInfo
   Application.php14.81%100%25%13.04%101, 105, 56–57, 60, 67, 69–71, 76–79, 81, 85–90
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
   ConfigController.php67.62%100%50%68.88%105, 122–123, 125, 127–129, 131–132, 137–138, 140, 416–419, 421–422, 425, 433, 444, 458–460, 475–479, 481–484, 502–509, 511, 513, 516–518, 532, 540–543, 545–548, 558–563
   DirectDownloadController.php0%100%0%0%36–38, 53–55, 57, 60–61
   DirectUploadController.php71.43%100%100%70.43%131–132, 177, 188, 192–195, 197, 207, 214, 230–232, 234–235, 238–240, 246, 248, 253–254, 261–262, 265–266, 269–270, 286–287, 307, 312, 318
   FilesController.php73.74%100%100%72.04%168, 221–222, 267, 273–277, 281–283, 285, 287, 298–300, 303–304, 306–307, 311–314, 317
   OpenProjectAPIController.php85.07%100%78.57%85.56%136, 171, 188–189, 193, 197, 199–204, 206, 215–216, 219, 221, 223–226, 228–229, 234, 253, 278, 95
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101–102, 104–108, 116, 123–124, 126, 128–129, 131–132, 134, 137–138, 140–141, 143, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
   OpenprojectErrorException.php100%100%100%100%
   OpenprojectFileNotUploadedException.php100%100%100%100%
   OpenprojectResponseException.php100%100%100%100%
   OpenprojectUnauthorizedUserException.php0%100%0%0%16
server/apps/integration_openproject/lib/Listener
   LoadSidebarScript.php0%100%0%0%101, 103–105, 107, 109, 111–112, 114–115, 117, 119, 72–78, 80–81, 83–84, 86–87, 93–94, 96–97, 99
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%47, 57, 60, 63, 67, 70, 73, 77–79, 81
   Version2310Date20230116153411.php0%100%0%0%46, 49–52, 54–56, 60, 64, 68, 72, 76, 81–82, 84
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%102, 109–110, 113–118, 120–121, 123–125, 128–129, 131–132, 136–141, 147–148, 150, 159–163, 171–179, 195–202, 211–216, 71–75, 82, 89, 97, 99
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php43.90%100%60%41.67%125–128, 131, 80–87, 89–93, 95–97
   DirectDownloadService.php88%100%100%86.96%65–66, 68
   DirectUploadService.php54.55%100%66.67%52.63%112, 118, 79–82, 84, 89, 91
   OauthService.php0%100%0%0%108–115, 45–47, 56–66, 68, 70–72, 75–78, 89, 91–94, 96–97
   OpenProjectAPIService.php78.06%100%83.33%77.62%113–117, 290–291, 293, 307–308, 317, 321, 342, 430–431, 438, 441–444, 446, 452, 456–458, 478, 486–489, 493–495, 500–502, 505–507, 509, 512–513, 516, 682, 753, 783–784, 786, 788–789, 812, 816, 820–821, 824–827, 829–830, 832–833, 835–836
server/apps/integration_openproject/lib/Settings
   Admin.php0%100%0%0%32–34, 41–43, 46–50, 53, 58–59, 62, 64–65, 67, 71, 75
   AdminSection.php0%100%0%0%19–20, 29, 39, 48, 55
   Personal.php87.88%100%50%93.10%94, 98
   PersonalSection.php0%100%0%0%19–20, 29, 39, 48, 55

@SwikritiT
Copy link
Contributor Author

SwikritiT commented Jul 28, 2023 •
edited
Loading

The CI errors are unrelated to this PR, it's because the activity app is not installed for stable 22, it gets downloaded sometimes other times not(this is more of a problem with the dev container that we are using). I cannot make all the pipelines pass in a single run but they do pass. I'm merging this PR to unblock the release

@SwikritiT SwikritiT merged commit 3df7e63 into release/2.3 Jul 28, 2023
@delete-merged-branch delete-merged-branch bot deleted the encrypt-the-oauth-client-secrets branch July 28, 2023 11:34
SwikritiT added a commit that referenced this pull request Jul 28, 2023
@SwikritiT
Encrypt the oauth client secrets (#446)
8167906 
* Encrypt the OAuth2 client secrets

check versions of nc

add migration step

update version and extensions

try running qb

disable migration

enable migration

enable migration

enable migration

enable migration

enable migration

remove migration

add more version checks

add more version checks

add migration

add migration

add extension

don't show secrets

update unit tests

fix api test

fix api test

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

* remove stable22 from api test

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

* bring back stable22 for api test

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>

---------

Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SagarGi SagarGi SagarGi approved these changes

@wielinde wielinde Awaiting requested review from wielinde

@julien-nc julien-nc Awaiting requested review from julien-nc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SwikritiT @SagarGi