-
-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
feat(passport): Settings - Add missing scopes to Application page #2351
Conversation
2c28984
to
1526f7a
Compare
@@ -458,6 +458,10 @@ export default function Authorize() { | |||
} | |||
} | |||
|
|||
if (requestedScope.includes("openid")) { | |||
personaData.openid = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to have something in personaData for openid scope when retriving it to display
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See other comment; openid
should not be shown, so it can be removed.
ba3bd1b
to
e5fadfd
Compare
@@ -77,6 +76,33 @@ export default () => { | |||
type: profile.type === 'eth' ? 'blockchain' : profile.type, | |||
})), | |||
}) | |||
} else if (scopeValue === "openid") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openid
is a system scope, with no associated claims. It can be removed.
@@ -458,6 +458,10 @@ export default function Authorize() { | |||
} | |||
} | |||
|
|||
if (requestedScope.includes("openid")) { | |||
personaData.openid = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See other comment; openid
should not be shown, so it can be removed.
packages/security/persona.ts
Outdated
@@ -320,6 +320,28 @@ async function profileClaimsRetriever( | |||
} else throw new InvalidPersonaDataError() | |||
} | |||
|
|||
async function openidClaimsRetriever( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove, as per other comments.
accounts={claim.accounts} | ||
connectedAccounts={true} | ||
/> | ||
case 'openid': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove, as per previous comments.
f992f33
to
280f086
Compare
}) | ||
} else if (scopeMeta[scopeValue].hidden) { | ||
aggregator.push({ | ||
claim: 'system_identifiers', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep it as 'claim' here to be consistent with other scopes from the list. Note that nothing is being shared as claim value in this case
Description
Adds missing scopes to application view in passport
Related Issues
Testing
Under
settings/applications/${clientId}
route in passport all visual changes were testedChecklist