-
Notifications
You must be signed in to change notification settings - Fork 556
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
[SDK-2708] Use configured domain for client assets download #2029
Conversation
This pull request introduces 1 alert and fixes 2 when merging 99c2fa2 into 1d3e7be - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 1 alert and fixes 2 when merging ddf3354 into 1d3e7be - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 1 alert and fixes 2 when merging 02687f6 into 1d3e7be - view on LGTM.com new alerts:
fixed alerts:
|
02687f6
to
1f04fcf
Compare
This pull request introduces 1 alert and fixes 2 when merging 1f04fcf into 1d3e7be - view on LGTM.com new alerts:
fixed alerts:
|
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.
LGTM!
expect(mock.calls.length).toBe(1); | ||
|
||
const model = mock.calls[0][1].toJS(); | ||
expect(model.tenantBaseUrl).toBe('https://auth.my-tenant.com/info-v1.js'); |
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.
why the path differs in this one? what's the condition? you might want to reflect that in the test name
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.
Yeah I'm not sure of the history as to why this is. If you use a custom domain, the URL differs when using __useTenantInfo
vs using an auth0.com
domain. Just trying to keep and verify the same logic here.
This pull request introduces 1 alert and fixes 2 when merging 33a4c71 into 1d3e7be - view on LGTM.com new alerts:
fixed alerts:
|
Changes
This PR reconfigures Lock to use the configured domain when constructing the URL to download the client assets file, instead of using
cdn.[locality].auth0.com
.The change applies to:
example.us.auth0.com
auth.custom-domain.com
__useTenantInfo: true
in the config__useTenantInfo: true
in the configTest coverage for these settings has been added.
Testing
Checklist