Skip to content

Conversation

isneezy
Copy link
Contributor

@isneezy isneezy commented Feb 18, 2020

I've fixed the White Label error with a simple filter by reusing the existing REGEX
The only catch up with this approach is that if you plan to have other URLs other than /api ex: /oauth etc you'll have to include them in the REGEX expression

Fixes #41

@coveralls
Copy link

Pull Request Test Coverage Report for Build 497

  • 18 of 18 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+6.4%) to 91.304%

Totals Coverage Status
Change from base Build 495: 6.4%
Covered Lines: 63
Relevant Lines: 69

💛 - Coveralls

@isneezy isneezy marked this pull request as ready for review February 18, 2020 14:32
@isneezy isneezy requested a review from jonashackt February 18, 2020 14:32

// Forwards all routes to FrontEnd except: '/', '/index.html', '/api', '/api/**'
// Required because of 'mode: history' usage in frontend routing, see README for further details
@RequestMapping(value = "{_:^(?!index\\.html|api).*$}")
Copy link

Choose a reason for hiding this comment

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

The forward in this rule did not work for two reasons

  1. it was placed int he wrong RequestMapping context as it would look for the pattern only after /api
  2. it was placed in a RestController instead of a Controller that does not support returning a forwad:/

Although the filter probably does work the easier solution would be to just fix 1) & 2)

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 info, I didn't knew about it! I'll see if it works and I'll update the PR in the mean time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hemeroc I've tried to follow your idea, you were right about the using Controller instead of RestController but unfortunately, this solution only works on one level URLs like this /login but not for this /auth/login or any URL that has many deep level path as you can see the tests here:

void redirectSpa() {
ValidatableResponse response = RestAssured.when().get("/path").then();
assertSpaResponse(response);
response = RestAssured.when().get("/nested/path").then();
assertSpaResponse(response);
response = RestAssured.when().get("/deep/nested/path/with/many/may/deep").then();
assertSpaResponse(response);
}

Do you have any idea on how to match URLs with multiple levels?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've found a way to accomplish using @Controller instead of Filter I'll update as soon as possible

Choose a reason for hiding this comment

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

I've found a way to accomplish using @Controller instead of Filter I'll update as soon as possible

when you will update :)

Copy link
Owner

Choose a reason for hiding this comment

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

@isneezy great contribution 👍 Sorry, didn't find the time earlier.

Copy link
Owner

@jonashackt jonashackt left a comment

Choose a reason for hiding this comment

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

Think that's great, I'll resolve the JUnit 4 issue that will occur, since there's no RunWith anymore in Spring Boot 2.4.x. Thanks again 👍


// Forwards all routes to FrontEnd except: '/', '/index.html', '/api', '/api/**'
// Required because of 'mode: history' usage in frontend routing, see README for further details
@RequestMapping(value = "{_:^(?!index\\.html|api).*$}")
Copy link
Owner

Choose a reason for hiding this comment

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

@isneezy great contribution 👍 Sorry, didn't find the time earlier.

@jonashackt jonashackt merged commit 5cd7a45 into jonashackt:master Feb 12, 2021
jonashackt added a commit that referenced this pull request Feb 12, 2021
…ixed SpaRedirectTest not Found message String.
jonashackt added a commit that referenced this pull request Feb 12, 2021
…tSpa:33->assertSpaResponse:58 1 expectation failed.

                                       Response body doesn't match expectation.
                                       Expected: a string containing "<script src=/static/js/chunk-vendors"
                                         Actual: <!DOCTYPE html><html lang="en"><head><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=edge"><meta name="viewport" content="width=device-width,initial-scale=1"><link rel="icon" href="/favicon.ico"><title>frontend</title><link href="/static/css/app.6dd46426.css" rel="preload" as="style"><link href="/static/css/chunk-vendors.2f24a82e.css" rel="preload" as="style"><link href="/static/js/app.5c8c692c.js" rel="preload" as="script"><link href="/static/js/chunk-vendors.f6c6f821.js" rel="preload" as="script"><link href="/static/css/chunk-vendors.2f24a82e.css" rel="stylesheet"><link href="/static/css/app.6dd46426.css" rel="stylesheet"></head><body><noscript><strong>We're sorry but frontend doesn't work properly without JavaScript enabled. Please enable it to continue.</strong></noscript><div id="app"></div><script src="/static/js/chunk-vendors.f6c6f821.js"></script><script src="/static/js/app.5c8c692c.js"></script></body></html>`
@isneezy isneezy deleted the fix-spa-redirect branch March 3, 2021 13:46
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.

404 - Not found when entered directly into the browser
5 participants