Skip to content
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

Merged
merged 8 commits into from
Sep 28, 2023

Conversation

thisisamir98
Copy link
Contributor

@thisisamir98 thisisamir98 commented Sep 18, 2023

BugWPB-1343 [Web] [OAuth] OAuth parameters are sent to webapp server for Authorization Prompt

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

  • PR has been self reviewed by the author;
  • Hard-to-understand areas of the code have been commented;
  • If it is a core feature, unit tests have been added;

…ry Parameters for Enhanced Security (WPB-1343)
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #15844 (af613df) into dev (2979a63) will increase coverage by 0.02%.
Report is 95 commits behind head on dev.
The diff coverage is 9.09%.

@@            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     

@thisisamir98 thisisamir98 marked this pull request as ready for review September 22, 2023 09:54
@@ -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}`});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

Copy link
Contributor Author

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

@tlebon
Copy link
Contributor

tlebon commented Sep 25, 2023

not working when signing out of account

@thisisamir98
Copy link
Contributor Author

@tlebon it is handled now in 102786c

@@ -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 => {
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@tlebon tlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@thisisamir98 thisisamir98 merged commit 6493135 into dev Sep 28, 2023
13 checks passed
@thisisamir98 thisisamir98 deleted the feat/WPB-1343 branch September 28, 2023 12:40
tlebon pushed a commit that referenced this pull request Oct 9, 2023
…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
Sign up for free to 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