Skip to content

fix(oidc): dedupe 'openid' scope in config#517

Open
tmlmt wants to merge 1 commit into
atinux:mainfrom
tmlmt:fix/dedupe-openid
Open

fix(oidc): dedupe 'openid' scope in config#517
tmlmt wants to merge 1 commit into
atinux:mainfrom
tmlmt:fix/dedupe-openid

Conversation

@tmlmt

@tmlmt tmlmt commented Apr 18, 2026

Copy link
Copy Markdown

Fixes: #516

Changes

Replaced the defu default [{ scope: ['openid'] }] with an explicit check that ensures 'openid' is present without duplication, consistent with how the Dropbox provider already handles it:

if (!config.scope.includes('openid')) {
config.scope.push('openid')
}

@ImBoop

ImBoop commented Apr 20, 2026

Copy link
Copy Markdown

It should probably actually just create a fresh object or only merge the config once. the config = defu(config,...) is modifying the original object every request so one could instead do config = defu({}, ...) instead (or again just do the merging beforehand such as before the return)

@tmlmt

tmlmt commented Apr 20, 2026

Copy link
Copy Markdown
Author

Interesting point but which should be raised in a separate issue as it is unrelated to the issue this PR is addressing.

@ImBoop

ImBoop commented Apr 20, 2026

Copy link
Copy Markdown

Interesting point but which should be raised in a separate issue as it is unrelated to the issue this PR is addressing.

It is related. The cause is overwriting config every request w/ defu (the first parameter being the object target) - if the user specifies 'openid' in their scopes list, your deduplication only prevents it from being added an additional time. The cleaner fix is to build it before the handler at all, which would fix your issue as well as the general duplicated scopes list issue.

Your bug is caused by the defu duplication; just applying your fix without the additional nuances I mentioned will only fix one particular instance of the broader problem; the problem will continue to occur.

@tmlmt

tmlmt commented Apr 21, 2026

Copy link
Copy Markdown
Author

What this PR fixes is a specific bug where 'openid' ends up duplicated in the scope list. The fix correctly replaces the defu default { scope: ['openid'] } approach with an explicit guard that only adds 'openid' if it's not already present. This directly and correctly solves issue #516.

What you are raising is a broader concern related to the fact that config = defu(config, ...) mutates the original config object on every request (since defu's first argument is the mutation target) but this does not address the target issue for this PR and would be worth elaborating in a separate issue.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OIDC provider: duplicate 'openid' scope due to defu array merging

2 participants