C-408 Send OAuth read-only response#1311
Conversation
|
Preview this change https://demo.audius.co/nkang--oauth-response |
sliptype
left a comment
There was a problem hiding this comment.
This is looking great! The redirect validation is definitely interesting bc we aren't registering the url, but good enough for now
| if (redirect_uri && typeof redirect_uri === 'string') { | ||
| let res: URL | ||
| try { | ||
| res = new URL(redirect_uri) |
There was a problem hiding this comment.
Can we just return here?
| res = new URL(redirect_uri) | |
| return new URL(redirect_uri) |
| const [password, setPassword] = useState('') | ||
| const [submitError, setSubmitError] = useState<string | null>(null) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
What's the reasoning behind making isRedirectValid a ref?
You could use useMemo here I believe:
| useEffect(() => { | |
| const isRedirectValid = useMemo(() => { |
There was a problem hiding this comment.
No real reason! I'll change it so it's more grokkable
| return | ||
| } | ||
| const statePart = state != null ? `state=${state}&` : '' | ||
| const fragment = `#${statePart}token=${jwt}` |
There was a problem hiding this comment.
Why put the token in the hash rather than as a query param?
There was a problem hiding this comment.
This is just convention from the OpenID spec which is what we're emulating with this flow
https://openid.net/specs/openid-connect-core-1_0.html#ImplicitFlowAuth
|
Preview this change https://demo.audius.co/nkang--oauth-response |
Description
Form JWT containing user's profile information to send back to the redirect URI.
Dragons
Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
How will this change be monitored?
For features that are critical or could fail silently please describe the monitoring/alerting being added.