Skip to content

[WP#57654]switch encryption for oauth secret #694

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
Merged
individual-it merged 4 commits into from
Sep 6, 2024
Merged

[WP#57654]switch encryption for oauth secret #694

individual-it merged 4 commits into from
Sep 6, 2024

Conversation

@SagarGi
Copy link
Contributor

@SagarGi SagarGi commented Sep 5, 2024 •
edited
Loading

Description

In OAuth, now the client secret are hashed for greater security which is done in this PR nextcloud/server#47635. Previously we just used to encrypt it.

This PR:

  • Makes changes for the hash update for different NC versions for our APP
  • Also remove client_secret when fetching the client info for the existing user.

Related Issue or Workpackage

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Updated CHANGELOG.md file

@SagarGi SagarGi force-pushed the changeEncryptionForOauthForNC branch 7 times, most recently from 8aa270c to 64675d9 Compare September 5, 2024 11:32
@SagarGi SagarGi changed the title switch encryption for oauth secret [WP#57654]switch encryption for oauth secret Sep 5, 2024
@SagarGi SagarGi force-pushed the changeEncryptionForOauthForNC branch from 64675d9 to b9231fc Compare September 5, 2024 11:45
@SagarGi SagarGi changed the base branch from master to release/2.7 September 5, 2024 11:45
@SagarGi SagarGi force-pushed the changeEncryptionForOauthForNC branch from b9231fc to 71c53dd Compare September 5, 2024 11:47
@SagarGi SagarGi marked this pull request as ready for review September 5, 2024 11:47
@SagarGi SagarGi self-assigned this Sep 5, 2024
@SagarGi SagarGi requested review from individual-it and nabim777 and removed request for individual-it September 5, 2024 11:47
individual-it
individual-it reviewed Sep 6, 2024
View reviewed changes
Copy link
Collaborator

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

there is more logic in createNcOauthClient so need to create unit tests for it

individual-it
individual-it requested changes Sep 6, 2024
View reviewed changes
@SagarGi
review address
1a45400 
Signed-off-by: Sagar <[email protected]>
@github-actions
Copy link

github-actions bot commented Sep 6, 2024

JS Code Coverage

Coverage after merging changeEncryptionForOauthForNC into release/2.7 will be
87.83%
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–25, 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
   reference.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60, 7–9
   utils.js71.43%33.33%50%73.85%10–14, 17–26, 6–9
src/components
   AdminSettings.vue96.16%93.83%95.16%96.52%1033–1036, 1072–1074, 1094–1097, 518–519, 655, 667–669, 681, 681, 681–686, 689–690, 694–695, 698–699, 703–704, 714–719, 779–781, 793–796, 809–811, 996–998
   OAuthConnectButton.vue91.54%75%100%92.98%59–64, 67–71
   PersonalSettings.vue90.16%94.44%85.71%89.88%100, 110–115, 118–127, 99
src/components/admin
   FieldValue.vue100%100%100%100%
   FormHeading.vue100%100%100%100%
   ProjectFolderError.vue100%100%100%100%
   TermsOfServiceUnsigned.vue100%100%100%100%
   TextInput.vue100%100%100%100%
src/components/icons
   ClippyIcon.vue100%100%100%100%
   OpenProjectIcon.vue100%100%100%100%
src/components/settings
   CheckBox.vue100%100%100%100%
   SettingsTitle.vue96.74%85.71%100%97.53%46–48
src/components/tab
   EmptyContent.vue98.90%95%100%99.35%92–93
   SearchInput.vue95.27%92.96%94.74%95.73%134–135, 188, 199–204, 263–265, 281–283, 287–292
   WorkPackage.vue86.10%73.17%93.33%87.46%102–111, 124–126, 137–141, 151–153, 171–177, 215, 215–220, 220, 220–231, 76–77
src/filesPlugin
   filesPlugin.js0%0%0%0%1, 1, 10, 100–104, 11–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–79, 8, 80–89, 9, 90–99
   filesPluginLessThan28.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–78, 8–9
src/utils
   workpackageHelper.js93.73%93.10%88.89%94.15%18–22, 49, 49–51, 94–99
src/views
   CreateWorkPackageModal.vue94.34%86.54%91.67%95.50%353–355, 358, 459–462, 467–472, 477–482, 488–491, 494, 510, 510, 550–554, 564–566, 585–587, 617–619, 641–643, 652–656
   Dashboard.vue77.40%80.39%61.90%78.01%103–108, 115, 119–120, 125, 128, 131–134, 139–141, 182–188, 194–197, 199–209, 238–246, 259–273, 51, 63, 88–91, 98–99
   LinkMultipleFilesModal.vue100%100%100%100%
   ProjectsTab.vue94.91%94.23%93.33%95.07%113–116, 142, 153–154, 188–198, 246–248

@github-actions
Copy link

github-actions bot commented Sep 6, 2024

PHP Code Coverage

Coverage after merging changeEncryptionForOauthForNC into release/2.7 will be
60.65%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib
   Capabilities.php0%100%0%0%19, 26–35
server/apps/integration_openproject/lib/AppInfo
   Application.php7.41%100%25%6%101, 103–106, 108, 110, 112, 116–119, 121–132, 134, 137, 141, 145–147, 74–76, 79–83, 85–90, 92–93, 96
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
   ConfigController.php64.76%100%50%65.37%135, 152–153, 155, 157–159, 161–164, 167–168, 170, 212, 222, 226–228, 292–296, 406–408, 410–412, 417–420, 461, 549–552, 554–555, 558, 566–570, 581, 595–598, 606–607, 611–614, 628–632, 634–635, 637–653, 670–675, 677–678, 680–682, 685, 687–703, 716–723, 725–728, 730–734, 743–748
   DirectDownloadController.php0%100%0%0%36–38, 53–55, 57–64
   DirectUploadController.php71.03%100%100%70.21%141–143, 187–189, 200, 204–207, 209, 219, 226, 242–244, 246–247, 250–255, 258, 260, 268–270, 276–278, 286–288, 303–305, 324, 329, 335
   FilesController.php72.95%100%83.33%72.41%181–182, 244, 249–252, 254, 256–269, 272–273, 275–276, 280–283, 286, 292
   OpenProjectAPIController.php85.60%100%80%85.92%139, 180, 204, 230–233, 236–243, 245–249, 251, 263, 272, 290, 299, 369, 371, 421, 423, 443, 445, 492, 494, 520–523, 526–530, 532, 737–741, 96
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101, 108, 115–116, 118, 120–137, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
   OpenprojectAvatarErrorException.php100%100%100%100%
   OpenprojectErrorException.php100%100%100%100%
   OpenprojectFileNotUploadedException.php100%100%100%100%
   OpenprojectGroupfolderSetupConflictException.php100%100%100%100%
   OpenprojectResponseException.php100%100%100%100%
   OpenprojectUnauthorizedUserException.php0%100%0%0%16
server/apps/integration_openproject/lib/Listener
   BeforeGroupDeletedListener.php0%100%0%0%48, 56–57, 60–63, 65
   BeforeNodeInsideOpenProjectGroupfilderChangedListener.php0%100%0%0%41–43, 47–50, 52, 54, 57–58, 60, 62–65, 67–70, 72–74
   BeforeUserDeletedListener.php0%100%0%0%48, 55–56, 58–61, 63
   LoadAdditionalScriptsListener.php0%100%0%0%17, 20–21, 24–26, 28
   LoadSidebarScript.php0%100%0%0%100, 102, 104–105, 107, 109, 111, 113–122, 75–91, 96–97, 99
   OpenProjectReferenceListener.php0%100%0%0%53–54, 57–58, 61–62, 64–67
   TermsOfServiceEventListener.php0%100%0%0%59–60, 65–66, 68–69, 71–73, 76–80
   UserChangedListener.php0%100%0%0%52, 59–60, 63–68, 71
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%47, 57–65, 67–75, 77–79, 81
   Version2310Date20230116153411.php0%100%0%0%46, 49–52, 54–79, 81–82, 84
   Version2400Date20230504144300.php0%100%0%0%47, 57–60
   Version2640Date20240628114301.php0%100%0%0%52, 64–66, 69–70, 73
server/apps/integration_openproject/lib/Reference
   WorkPackageReferenceProvider.php51.67%100%25%58.33%102, 108–111, 114–116, 119, 123, 157, 165–166, 174, 52, 59, 66, 73–75
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%103–104, 107–118, 121–122, 124–125, 128–137, 139–143, 66–69, 76, 83, 91, 93, 96
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php42.31%100%60%40.43%100–102, 125–129, 131, 80–93, 95–99
   DirectDownloadService.php88.46%100%100%87.50%65–66, 68
   DirectUploadService.php42.86%100%66.67%40%112, 118, 79–82, 84–92
   OauthService.php28.30%100%40%27.08%104–112, 123–130, 64–65, 79–87, 89–95
   OpenProjectAPIService.php75.66%100%75.93%75.64%1036–1038, 1040–1042, 1045–1049, 1051–1053, 1062–1065, 1068–1070, 1072, 1075–1080, 1084–1085, 1116–1119, 1138–1145, 1154–1155, 1162–1164, 1166–1169, 1173, 1182, 1200, 1202–1205, 1207–1212, 1354, 1366, 1369, 1391, 1394, 1404–1409, 1534–1536, 1538–1539, 1543–1544, 1546, 1548, 180–184, 255, 392–393,

@SagarGi
Copy link
Contributor Author

SagarGi commented Sep 6, 2024

API test is failing because of the flaky test https://github.com/nextcloud/integration_openproject/actions/runs/10729539649.

@individual-it individual-it merged commit 8d3d129 into release/2.7 Sep 6, 2024
@individual-it individual-it deleted the changeEncryptionForOauthForNC branch September 6, 2024 11:27
nabim777 pushed a commit that referenced this pull request Sep 9, 2024
@SagarGi @nabim777
[WP#57654]switch encryption for oauth secret (#694)
ac04ef0 
Signed-off-by: nabim777 <[email protected]>
SagarGi added a commit that referenced this pull request Sep 10, 2024
@SagarGi
[WP#57654]switch encryption for oauth secret (#694)
b49283d
@SagarGi SagarGi mentioned this pull request Sep 10, 2024
Merged
8 tasks
SagarGi added a commit that referenced this pull request Sep 10, 2024
@SagarGi @nabim777
@nickvergessen @individual-it
Merge 2.7.0 into master (#698)
0ab3c94 
* Drop support for nc 26 (#689)

* Adjust or remove code specific to NC-26

Signed-off-by: Sagar <[email protected]>

* Adjust or remove things realted to stable 26

Signed-off-by: Sagar <[email protected]>

* fix CI failure

Signed-off-by: Sagar <[email protected]>

* update changelog

Signed-off-by: Sagar <[email protected]>

---------

Signed-off-by: Sagar <[email protected]>

* Remove version comparision for secret encryption (#695)

* Remove version comparision for secret encryption

Signed-off-by: Sagar <[email protected]>

* stlye fix

Signed-off-by: Sagar <[email protected]>

---------

Signed-off-by: Sagar <[email protected]>

* [WP#56328]Fix error message when upload is restricted by file access control in `Nextcloud` (#688) (#696)

* Fix the misleading error message when upload is restricted by file access control app



* Added php unit test



* update change log



---------

Signed-off-by: Sagar <[email protected]>
Signed-off-by: nabim777 <[email protected]>
Co-authored-by: Sagar Gurung <[email protected]>

* [WP#57654]switch encryption for oauth secret (#694)

* ci: Reduce testing matrix

Signed-off-by: Joas Schilling <[email protected]>

* CI: Run phpunit test one times more on error (#687)

* retry on the phpunit error for ci

Signed-off-by: nabim777 <[email protected]>

* use pipeline for retry

Signed-off-by: nabim777 <[email protected]>

---------

Signed-off-by: nabim777 <[email protected]>

---------

Signed-off-by: Sagar <[email protected]>
Signed-off-by: nabim777 <[email protected]>
Signed-off-by: Joas Schilling <[email protected]>
Co-authored-by: Nalem7 <[email protected]>
Co-authored-by: Joas Schilling <[email protected]>
Co-authored-by: Artur Neumann <[email protected]>
@github-actions
Copy link

github-actions bot commented Sep 20, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@individual-it individual-it individual-it approved these changes

@nabim777 nabim777 Awaiting requested review from nabim777

Assignees

@SagarGi SagarGi

Labels

feedback-requested

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SagarGi @individual-it