-
Notifications
You must be signed in to change notification settings - Fork 289
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
fix: Update Authorization Prompt URL to Use URL Anchor Instead of Query Parameters (WPB-1343) #15844
Conversation
…ry Parameters for Enhanced Security (WPB-1343)
Codecov Report
@@ Coverage Diff @@
## dev #15844 +/- ##
==========================================
+ Coverage 44.48% 44.51% +0.02%
==========================================
Files 673 674 +1
Lines 22719 22774 +55
Branches 5166 5180 +14
==========================================
+ Hits 10107 10138 +31
- Misses 11324 11339 +15
- Partials 1288 1297 +9 |
src/script/auth/page/Login.tsx
Outdated
@@ -182,7 +182,8 @@ const LoginComponent = ({ | |||
await doInitializeClient(ClientType.PERMANENT, undefined, undefined, entropyData); | |||
|
|||
if (isOauth) { | |||
return navigate(ROUTE.AUTHORIZE); | |||
console.info('bardia route', {route: `${ROUTE.AUTHORIZE}/${window.location.hash}`}); |
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.
👀
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.
thanks for the catch: 158a9b9
not working when signing out of account |
src/script/auth/page/Root.tsx
Outdated
@@ -107,7 +107,20 @@ const RootComponent: FC<RootProps & ConnectedProps & DispatchProps> = ({ | |||
}; | |||
|
|||
const isAuthenticatedCheck = (page: any): any => (page ? (isAuthenticated ? page : navigate('/auth')) : null); | |||
const isOAuthCheck = (page: any): any => (page ? isAuthenticated ? page : <Navigate to={ROUTE.LOGIN} /> : null); | |||
const isOAuthCheck = (page: any): any => { |
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 don't know this type?
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.
don't know, its been like this forever
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.
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.
looks good
…ry Parameters (WPB-1343) (#15844) * fix: Update Authorization Prompt URL to Use URL Anchor Instead of Query Parameters for Enhanced Security (WPB-1343) * replace extra info on url * remove extra log * handle signout * change location search to hash * remove any type * better route handling * extract logic
Description
This PR modifies the Authorization Prompt URL to add URL anchors to OAuth URLs. By doing so, we ensure that potentially sensitive data, which is solely managed by client-side, is no longer sent to the backend server.
Screenshots/Screencast (for UI changes)
bbb.MP4
Checklist