Skip to content

[oauth2] Store hashed secret instead of encrypted - #47635

Merged
julien-nc merged 3 commits into
masterfrom
fix/noid/oauth2-store-secret-hash
Sep 2, 2024
Merged

[oauth2] Store hashed secret instead of encrypted#47635
julien-nc merged 3 commits into
masterfrom
fix/noid/oauth2-store-secret-hash

Conversation

@julien-nc

Copy link
Copy Markdown
Member

Store the oauth2 secrets as hash instead of encrypting them. Then they can be validated but not recovered.

UI adjustment:

image

  • Show the eye button only if necessary
  • Display a warning when a client is added (when a secret is actually displayed)

@julien-nc

Copy link
Copy Markdown
MemberAuthor

/compile

@julien-nc

Copy link
Copy Markdown
MemberAuthor

/backport to stable30

@julien-nc

Copy link
Copy Markdown
MemberAuthor

/backport to stable29

@julien-nc

Copy link
Copy Markdown
MemberAuthor

/backport to stable28

@julien-nc

Copy link
Copy Markdown
MemberAuthor

/backport to stable27

try {
$storedClientSecret = $this->crypto->decrypt($client->getSecret());
$storedClientSecretHash = $client->getSecret();
$clientSecretHash = bin2hex($this->crypto->calculateHMAC($client_secret));

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of OCP\Security\ICrypto::calculateHMAC cannot be null, possibly null value provided
Comment threadapps/oauth2/lib/Migration/Version011901Date20240829164356.php Fixed
@julien-nc
julien-ncforce-pushed the fix/noid/oauth2-store-secret-hash branch from cd765c2 to 43ae7d2CompareAugust 30, 2024 13:11
@julien-nc
julien-nc enabled auto-merge August 30, 2024 14:09
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc
julien-ncforce-pushed the fix/noid/oauth2-store-secret-hash branch from 43ae7d2 to 120e7e8CompareSeptember 2, 2024 12:39
@julien-nc

Copy link
Copy Markdown
MemberAuthor

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@julien-nc
julien-nc merged commit 09b8aea into masterSep 2, 2024
@julien-nc
julien-nc deleted the fix/noid/oauth2-store-secret-hash branch September 2, 2024 13:28
@backportbot

Copy link
Copy Markdown

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27
# Create the new backport branch
git checkout -b backport/47635/stable27
# Cherry pick the change from the commit sha1 of the change against the default branch# This might cause conflicts, resolve them
git cherry-pick 034917b7 120e7e83 5b98abcf
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/47635/stable27

Error: Failed to clone repository: Failed to checkout branches: Updating 1cc7851..09b8aea
From https://github.com/nextcloud/server

  • [deleted] (none) -> origin/7935-download-files-via-post
  • [deleted] (none) -> origin/JonathanTreffler-stop-spamming-deprecations
  • [deleted] (none) -> origin/add-clear-add-to-user-status-public-api
  • [deleted] (none) -> origin/add-integration-tests-for-getting-folder-sizes
  • [deleted] (none) -> origin/add-vtimezone-data-when-creating-personal-calendar
  • [deleted] (none) -> origin/admin_audit/enh/move-to-event-listeners
  • [deleted] (none) -> origin/artonge/feat/download_providers
  • [deleted] (none) -> origin/bug/files-scroll
  • [deleted] (none) -> origin/bugfix/38171/revert-status-when-overwritten
  • [deleted] (none) -> origin/bugfix/avoid-extra-stream-copy
  • [deleted] (none) -> origin/bugfix/noid/unavailable-shares
  • [deleted] (none) -> origin/cache-mimtype-mapping
  • [deleted] (none) -> origin/chore/catch-missing-non-optional-controller-parameter
  • [deleted] (none) -> origin/chore/security/log-password-confirmation-user-backend
  • [deleted] (none) -> origin/dependabot/npm_and_yarn/core-js-3.38.1
  • [deleted] (none) -> origin/direct-access-shared-calendar
  • [deleted] (none) -> origin/email-template-html-fragment
  • [deleted] (none) -> origin/enh/a11y-util
  • [deleted] (none) -> origin/enh/noid/iconfig
  • [deleted] (none) -> origin/enh/noid/use-taskprocessing-in-old-managers
  • [deleted] (none) -> origin/enhancement/caldav-resources-sync-command
  • [deleted] (none) -> origin/enhancement/typed-db-entity
  • [deleted] (none) -> origin/external-list-for
  • [deleted] (none) -> origin/extract-caldav-sharing-plugin
  • [deleted] (none) -> origin/feat-add-iavaialble-in-maintenance-mode
  • [deleted] (none) -> origin/feat/mail-admin-vue
  • [deleted] (none) -> origin/file-cache-insertion-atomic
  • [deleted] (none) -> origin/fix/44160/ctrl-f-trigger-global-search
  • [deleted] (none) -> origin/fix/files-non-visual-loading-info
  • [deleted] (none) -> origin/fix/header-styles
  • [deleted] (none) -> origin/fix/noid/identity-proof-key-checksum
  • [deleted] (none) -> origin/fix/user-settings-admin
  • [deleted] (none) -> origin/home-storage-lazy-datadir
  • [deleted] (none) -> origin/introduce/orm
  • [deleted] (none) -> origin/locate-key-fix
  • [deleted] (none) -> origin/mountcache-lazy-user
  • [deleted] (none) -> origin/multi-object-store
  • [deleted] (none) -> origin/remove_depreated_files
  • [deleted] (none) -> origin/setupmanager-lazy-user
  • [deleted] (none) -> origin/share-null-source
  • [deleted] (none) -> origin/storage-cache-init-in-transaction
  • [deleted] (none) -> origin/trashbin-skip-logging
  • [deleted] (none) -> origin/upload-chunk-locking
  • [deleted] (none) -> origin/use_HSTS
  • [deleted] (none) -> origin/user-files-debug-info
  • [deleted] (none) -> origin/work/sharing_trash
    error: Your local changes to the following files would be overwritten by merge:
    dist/4254-4254.js.map.license
    dist/4696-4696.js.map.license
    dist/5643-5643.js.map.license
    Please commit your changes or stash them before you merge.
    Aborting

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot

Copy link
Copy Markdown

The backport to stable28 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable28
git pull origin stable28
# Create the new backport branch
git checkout -b backport/47635/stable28
# Cherry pick the change from the commit sha1 of the change against the default branch# This might cause conflicts, resolve them
git cherry-pick 034917b7 120e7e83 5b98abcf
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/47635/stable28

Error: Failed to clone repository: Failed to checkout branches: Updating 1cc7851..09b8aea
From https://github.com/nextcloud/server

  • [deleted] (none) -> origin/7935-download-files-via-post
  • [deleted] (none) -> origin/JonathanTreffler-stop-spamming-deprecations
  • [deleted] (none) -> origin/add-clear-add-to-user-status-public-api
  • [deleted] (none) -> origin/add-integration-tests-for-getting-folder-sizes
  • [deleted] (none) -> origin/add-vtimezone-data-when-creating-personal-calendar
  • [deleted] (none) -> origin/admin_audit/enh/move-to-event-listeners
  • [deleted] (none) -> origin/artonge/feat/download_providers
  • [deleted] (none) -> origin/bug/files-scroll
  • [deleted] (none) -> origin/bugfix/38171/revert-status-when-overwritten
  • [deleted] (none) -> origin/bugfix/avoid-extra-stream-copy
  • [deleted] (none) -> origin/bugfix/noid/unavailable-shares
  • [deleted] (none) -> origin/cache-mimtype-mapping
  • [deleted] (none) -> origin/chore/catch-missing-non-optional-controller-parameter
  • [deleted] (none) -> origin/chore/security/log-password-confirmation-user-backend
  • [deleted] (none) -> origin/dependabot/npm_and_yarn/core-js-3.38.1
  • [deleted] (none) -> origin/direct-access-shared-calendar
  • [deleted] (none) -> origin/email-template-html-fragment
  • [deleted] (none) -> origin/enh/a11y-util
  • [deleted] (none) -> origin/enh/noid/iconfig
  • [deleted] (none) -> origin/enh/noid/use-taskprocessing-in-old-managers
  • [deleted] (none) -> origin/enhancement/caldav-resources-sync-command
  • [deleted] (none) -> origin/enhancement/typed-db-entity
  • [deleted] (none) -> origin/external-list-for
  • [deleted] (none) -> origin/extract-caldav-sharing-plugin
  • [deleted] (none) -> origin/feat-add-iavaialble-in-maintenance-mode
  • [deleted] (none) -> origin/feat/mail-admin-vue
  • [deleted] (none) -> origin/file-cache-insertion-atomic
  • [deleted] (none) -> origin/fix/44160/ctrl-f-trigger-global-search
  • [deleted] (none) -> origin/fix/files-non-visual-loading-info
  • [deleted] (none) -> origin/fix/header-styles
  • [deleted] (none) -> origin/fix/noid/identity-proof-key-checksum
  • [deleted] (none) -> origin/fix/user-settings-admin
  • [deleted] (none) -> origin/home-storage-lazy-datadir
  • [deleted] (none) -> origin/introduce/orm
  • [deleted] (none) -> origin/locate-key-fix
  • [deleted] (none) -> origin/mountcache-lazy-user
  • [deleted] (none) -> origin/multi-object-store
  • [deleted] (none) -> origin/remove_depreated_files
  • [deleted] (none) -> origin/setupmanager-lazy-user
  • [deleted] (none) -> origin/share-null-source
  • [deleted] (none) -> origin/storage-cache-init-in-transaction
  • [deleted] (none) -> origin/trashbin-skip-logging
  • [deleted] (none) -> origin/upload-chunk-locking
  • [deleted] (none) -> origin/use_HSTS
  • [deleted] (none) -> origin/user-files-debug-info
  • [deleted] (none) -> origin/work/sharing_trash
    error: Your local changes to the following files would be overwritten by merge:
    dist/4254-4254.js.map.license
    dist/4696-4696.js.map.license
    dist/5643-5643.js.map.license
    Please commit your changes or stash them before you merge.
    Aborting

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot

Copy link
Copy Markdown

The backport to stable29 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable29
git pull origin stable29
# Create the new backport branch
git checkout -b backport/47635/stable29
# Cherry pick the change from the commit sha1 of the change against the default branch# This might cause conflicts, resolve them
git cherry-pick 034917b7 120e7e83 5b98abcf
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/47635/stable29

Error: Failed to clone repository: Failed to checkout branches: Updating 1cc7851..09b8aea
From https://github.com/nextcloud/server

  • [deleted] (none) -> origin/7935-download-files-via-post
  • [deleted] (none) -> origin/JonathanTreffler-stop-spamming-deprecations
  • [deleted] (none) -> origin/add-clear-add-to-user-status-public-api
  • [deleted] (none) -> origin/add-integration-tests-for-getting-folder-sizes
  • [deleted] (none) -> origin/add-vtimezone-data-when-creating-personal-calendar
  • [deleted] (none) -> origin/admin_audit/enh/move-to-event-listeners
  • [deleted] (none) -> origin/artonge/feat/download_providers
  • [deleted] (none) -> origin/bug/files-scroll
  • [deleted] (none) -> origin/bugfix/38171/revert-status-when-overwritten
  • [deleted] (none) -> origin/bugfix/avoid-extra-stream-copy
  • [deleted] (none) -> origin/bugfix/noid/unavailable-shares
  • [deleted] (none) -> origin/cache-mimtype-mapping
  • [deleted] (none) -> origin/chore/catch-missing-non-optional-controller-parameter
  • [deleted] (none) -> origin/chore/security/log-password-confirmation-user-backend
  • [deleted] (none) -> origin/dependabot/npm_and_yarn/core-js-3.38.1
  • [deleted] (none) -> origin/direct-access-shared-calendar
  • [deleted] (none) -> origin/email-template-html-fragment
  • [deleted] (none) -> origin/enh/a11y-util
  • [deleted] (none) -> origin/enh/noid/iconfig
  • [deleted] (none) -> origin/enh/noid/use-taskprocessing-in-old-managers
  • [deleted] (none) -> origin/enhancement/caldav-resources-sync-command
  • [deleted] (none) -> origin/enhancement/typed-db-entity
  • [deleted] (none) -> origin/external-list-for
  • [deleted] (none) -> origin/extract-caldav-sharing-plugin
  • [deleted] (none) -> origin/feat-add-iavaialble-in-maintenance-mode
  • [deleted] (none) -> origin/feat/mail-admin-vue
  • [deleted] (none) -> origin/file-cache-insertion-atomic
  • [deleted] (none) -> origin/fix/44160/ctrl-f-trigger-global-search
  • [deleted] (none) -> origin/fix/files-non-visual-loading-info
  • [deleted] (none) -> origin/fix/header-styles
  • [deleted] (none) -> origin/fix/noid/identity-proof-key-checksum
  • [deleted] (none) -> origin/fix/user-settings-admin
  • [deleted] (none) -> origin/home-storage-lazy-datadir
  • [deleted] (none) -> origin/introduce/orm
  • [deleted] (none) -> origin/locate-key-fix
  • [deleted] (none) -> origin/mountcache-lazy-user
  • [deleted] (none) -> origin/multi-object-store
  • [deleted] (none) -> origin/remove_depreated_files
  • [deleted] (none) -> origin/setupmanager-lazy-user
  • [deleted] (none) -> origin/share-null-source
  • [deleted] (none) -> origin/storage-cache-init-in-transaction
  • [deleted] (none) -> origin/trashbin-skip-logging
  • [deleted] (none) -> origin/upload-chunk-locking
  • [deleted] (none) -> origin/use_HSTS
  • [deleted] (none) -> origin/user-files-debug-info
  • [deleted] (none) -> origin/work/sharing_trash
    error: Your local changes to the following files would be overwritten by merge:
    dist/4254-4254.js.map.license
    dist/4696-4696.js.map.license
    dist/5643-5643.js.map.license
    Please commit your changes or stash them before you merge.
    Aborting

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot

Copy link
Copy Markdown

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30
# Create the new backport branch
git checkout -b backport/47635/stable30
# Cherry pick the change from the commit sha1 of the change against the default branch# This might cause conflicts, resolve them
git cherry-pick 034917b7 120e7e83 5b98abcf
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/47635/stable30

Error: Failed to clone repository: Failed to checkout branches: Updating 1cc7851..09b8aea
From https://github.com/nextcloud/server

  • [deleted] (none) -> origin/7935-download-files-via-post
  • [deleted] (none) -> origin/JonathanTreffler-stop-spamming-deprecations
  • [deleted] (none) -> origin/add-clear-add-to-user-status-public-api
  • [deleted] (none) -> origin/add-integration-tests-for-getting-folder-sizes
  • [deleted] (none) -> origin/add-vtimezone-data-when-creating-personal-calendar
  • [deleted] (none) -> origin/admin_audit/enh/move-to-event-listeners
  • [deleted] (none) -> origin/artonge/feat/download_providers
  • [deleted] (none) -> origin/bug/files-scroll
  • [deleted] (none) -> origin/bugfix/38171/revert-status-when-overwritten
  • [deleted] (none) -> origin/bugfix/avoid-extra-stream-copy
  • [deleted] (none) -> origin/bugfix/noid/unavailable-shares
  • [deleted] (none) -> origin/cache-mimtype-mapping
  • [deleted] (none) -> origin/chore/catch-missing-non-optional-controller-parameter
  • [deleted] (none) -> origin/chore/security/log-password-confirmation-user-backend
  • [deleted] (none) -> origin/dependabot/npm_and_yarn/core-js-3.38.1
  • [deleted] (none) -> origin/direct-access-shared-calendar
  • [deleted] (none) -> origin/email-template-html-fragment
  • [deleted] (none) -> origin/enh/a11y-util
  • [deleted] (none) -> origin/enh/noid/iconfig
  • [deleted] (none) -> origin/enh/noid/use-taskprocessing-in-old-managers
  • [deleted] (none) -> origin/enhancement/caldav-resources-sync-command
  • [deleted] (none) -> origin/enhancement/typed-db-entity
  • [deleted] (none) -> origin/external-list-for
  • [deleted] (none) -> origin/extract-caldav-sharing-plugin
  • [deleted] (none) -> origin/feat-add-iavaialble-in-maintenance-mode
  • [deleted] (none) -> origin/feat/mail-admin-vue
  • [deleted] (none) -> origin/file-cache-insertion-atomic
  • [deleted] (none) -> origin/fix/44160/ctrl-f-trigger-global-search
  • [deleted] (none) -> origin/fix/files-non-visual-loading-info
  • [deleted] (none) -> origin/fix/header-styles
  • [deleted] (none) -> origin/fix/noid/identity-proof-key-checksum
  • [deleted] (none) -> origin/fix/user-settings-admin
  • [deleted] (none) -> origin/home-storage-lazy-datadir
  • [deleted] (none) -> origin/introduce/orm
  • [deleted] (none) -> origin/locate-key-fix
  • [deleted] (none) -> origin/mountcache-lazy-user
  • [deleted] (none) -> origin/multi-object-store
  • [deleted] (none) -> origin/remove_depreated_files
  • [deleted] (none) -> origin/setupmanager-lazy-user
  • [deleted] (none) -> origin/share-null-source
  • [deleted] (none) -> origin/storage-cache-init-in-transaction
  • [deleted] (none) -> origin/trashbin-skip-logging
  • [deleted] (none) -> origin/upload-chunk-locking
  • [deleted] (none) -> origin/use_HSTS
  • [deleted] (none) -> origin/user-files-debug-info
  • [deleted] (none) -> origin/work/sharing_trash
    error: Your local changes to the following files would be overwritten by merge:
    dist/4254-4254.js.map.license
    dist/4696-4696.js.map.license
    dist/5643-5643.js.map.license
    Please commit your changes or stash them before you merge.
    Aborting

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@julien-nc@AndyScherzinger@ChristophWurst@github-advanced-security@nextcloud-command