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

feat(admin-ui): Redirect to the right page on forbidden errors if an internal loginUrl is provided #2175

Conversation

gdarchen
Copy link
Contributor

🎯 Goal

This PR closes #2136.

🧠 Approach

We are currently facing an issue with the AdminUI.
Indeed, we configure a loginUrl towards a custom page which is internal (it shares the same origin as the rest of the AdminUI pages).

Therefore, when admins click on a link and face a FORBIDDEN error, they used to be totally redirected towards this loginUrl with a window.location.href = loginUrl, without the ?redirectTo query parameter.

In this PR, I made sure to check if the loginUrl is on the same origin than the AdminUI (window.location.origin). If that is the case, we redirect to that internal loginUrl and provide the ?redirectTo query parameter.

🧪 Test

  1. In the vendure-ui-config.json file, add the following to define a custom loginUrl:
    "loginUrl": "http://localhost:4200/admin/login",
  2. Run the dev server:
    cd packages/dev-server
    yarn start
  3. In another terminal, run the AdminUI:
    cd packages/admin-ui
    yarn start
  4. Connect on the AdminUI on http://localhost:4200/admin/login
  5. Clear your cookies and local storage
  6. Click on a menu item (such as Facets)
  7. A forbidden message should be displayed
  8. You should be redirected to the login page
  9. Fill in the login form again
  10. You should directly be redirected on the right page (Facets in my example)

@gdarchen
Copy link
Contributor Author

Hey @michaelbromley 👋
Is it possible in any way to retrieve the route parameter from the AdminUiPlugin in the interceptor? This would enable not to hard code the /admin route there 😊

@michaelbromley
Copy link
Member

@gdarchen Hi, and thanks for this PR!

I've not yet had time to properly review and test as I've been busy with some other things, but I just wanted to let you know it is not being ignored 👍

No, there is no way to access the config of the AdminUiPlugin from the interceptor currently. Maybe there is some other way to avoid the hard-coded admin string. When I get a moment to test out this PR I'll have a think about that.

@gdarchen
Copy link
Contributor Author

gdarchen commented Jun 6, 2023

Hey @michaelbromley 👋
I know you are working really hard on the v2 and we are looking forward for being able to migrate!
When do you think you will find a few minutes to check this PR please? It would make requests handling easier for our customer care teams 🙏

@michaelbromley
Copy link
Member

Hi,
question: does your team need this also on v1? Or are you planning to move to v2 soon?

@gdarchen
Copy link
Contributor Author

Hi @michaelbromley!
Yes are planning to move on the v2 shortly (and we are quite excited about it 🚀).
So indeed, you do not have to release a minor version in the v1 to integrate this change (unless you do it for other fixes).
But in the v2, I think the issue will be the same and this PR should enable to solve it, am I right?

@michaelbromley michaelbromley merged commit c0630fb into vendure-ecommerce:master Jun 13, 2023
@michaelbromley
Copy link
Member

Thanks! This will be in the patch being released today 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AdminUI] Redirect to a given page after login
2 participants