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

auth: Inform the user that the app origin is incorrect #3223

Closed
Rugvip opened this issue Nov 3, 2020 · 7 comments · Fixed by #3430
Closed

auth: Inform the user that the app origin is incorrect #3223

Rugvip opened this issue Nov 3, 2020 · 7 comments · Fixed by #3430
Labels
auth bug Something isn't working good first issue Good for newcomers help wanted Help/Contributions wanted from community members

Comments

@Rugvip
Copy link
Member

Rugvip commented Nov 3, 2020

Expected Behavior

The error dialog in some way informs the user that the app.baseUrl is incorrect

Current Behavior

The error dialog says that auth failed be cause the "popup was closed".

Possible Solution

Detect incorrect origins in the script returned from /api/auth/<provider>/handler/frame, and pass on a more descriptive error from the popup with postMessage

Steps to Reproduce

Set up any auth provider correctly, but use an app.baseUrl that does not match the origin that the app is hosted at.

@Rugvip Rugvip added bug Something isn't working auth good first issue Good for newcomers help wanted Help/Contributions wanted from community members labels Nov 3, 2020
@erictnilsson
Copy link
Contributor

I'd like to take a look at this, could you assign me @Rugvip ?🙂

@Rugvip
Copy link
Member Author

Rugvip commented Nov 8, 2020

@erictnilsson sure, thanks! 😁

@erictnilsson erictnilsson removed their assignment Nov 16, 2020
@erictnilsson
Copy link
Contributor

Hey! I've unassigned myself from this issue @Rugvip, I'm swamped with school work, can't really find time to work on this :/

@Rugvip
Copy link
Member Author

Rugvip commented Nov 16, 2020

@erictnilsson alright, no problem, thank you for the update!

@jot-hub
Copy link
Contributor

jot-hub commented Nov 18, 2020

Hi @Rugvip , I had a look at this. I am looking at https://github.com/backstage/backstage/blob/master/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts#L41-L46 but need a bit of help on the following:

  1. under what conditions would "app.baseUrl" actually differ here? I see that it is taken from https://github.com/backstage/backstage/blob/master/app-config.yaml#L3 or any such env specific config
  2. If I reproduce the problem by modifying baseUrl here https://github.com/backstage/backstage/blob/master/plugins/auth-backend/src/service/router.ts#L46, it will fail the postMessage (differing origin) mentioned above.
  3. How to change baseUrl but still do postMessage successfully with an error message?

postMessage fails silently, so can't handle errors there. any hints?

@Rugvip
Copy link
Member Author

Rugvip commented Nov 20, 2020

@jot-hub Thanks!

The things that can differ are the origin in app.baseUrl and the actual origin of the app. It's an error that will only happen when configuration is incorrect, but that'll happen sometimes and we want it to be as easy to detect as possible 😁

I'm thinking the simplest way to handle this might be to post two messages from the auth frame. First the auth response message that already exists, and then a second message that is posted with target origin '*'. If the second message is received but not the first, we know that something's wrong. The second message would contain no sensitive information, as posting the auth response to a specific origin is important for security. The second messages would just be something that has a similar format, but that can easily be identified and probably contains some additional information like the expected origin.

@jot-hub
Copy link
Contributor

jot-hub commented Nov 24, 2020

@Rugvip cool, took a dig at this ⬆️ - one thing to note is the order of the messages is different - please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth bug Something isn't working good first issue Good for newcomers help wanted Help/Contributions wanted from community members
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants