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

[SDK-2161] Allow to skip the redirect callback #86

Merged
merged 12 commits into from
Dec 2, 2020
7 changes: 7 additions & 0 deletions projects/auth0-angular/src/lib/auth.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ export interface AuthConfig {
*/
redirectUri?: string;

/**
* By default, if the page url has code/state params, the SDK will treat them as Auth0's and attempt to exchange the
* code for a token. In some cases the code might be for something else (another OAuth SDK perhaps). In these
* instances you can instruct the client to ignore them.
*/
skipRedirectCallback?: boolean;

/**
* The value in seconds used to account for clock skew in JWT expirations.
* Typically, this value is no more than a minute or two at maximum.
Expand Down
21 changes: 21 additions & 0 deletions projects/auth0-angular/src/lib/auth.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Auth0Client, IdToken } from '@auth0/auth0-spa-js';
import { AbstractNavigator } from './abstract-navigator';
import { filter } from 'rxjs/operators';
import { Location } from '@angular/common';
import { AuthClientConfig } from './auth.config';

/**
* Wraps service.isLoading$ so that assertions can be made
Expand All @@ -19,6 +20,7 @@ describe('AuthService', () => {
let moduleSetup: any;
let service: AuthService;
let locationSpy: jasmine.SpyObj<Location>;
let authClientConfigSpy: jasmine.SpyObj<AuthClientConfig>;

const createService = () => {
return TestBed.inject(AuthService);
Expand Down Expand Up @@ -47,6 +49,8 @@ describe('AuthService', () => {
locationSpy = jasmine.createSpyObj('Location', ['path']);
locationSpy.path.and.returnValue('');

authClientConfigSpy = jasmine.createSpyObj(['get']);

moduleSetup = {
providers: [
AbstractNavigator,
Expand Down Expand Up @@ -172,6 +176,10 @@ describe('AuthService', () => {
provide: Location,
useValue: locationSpy,
},
{
provide: AuthClientConfig,
useValue: authClientConfigSpy,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about our other conversations we've had about mocking less, do we need to use a Spy here or can we just give it an an instance of AuthClientConfig with the right config?

Copy link
Member Author

Choose a reason for hiding this comment

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

As the config is different between tests, mocking it makes it a bit easier I think.

],
});

Expand All @@ -187,6 +195,19 @@ describe('AuthService', () => {
});
});

it('should not handle the callback when skipRedirectCallback is true', (done) => {
authClientConfigSpy.get.and.returnValue({
skipRedirectCallback: true,
} as any);

const localService = createService();

loaded(localService).subscribe(() => {
expect(auth0Client.handleRedirectCallback).not.toHaveBeenCalledTimes(1);
frederikprijck marked this conversation as resolved.
Show resolved Hide resolved
done();
});
});

it('should redirect to the correct route', (done) => {
const localService = createService();

Expand Down
12 changes: 8 additions & 4 deletions projects/auth0-angular/src/lib/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
import { Auth0ClientService } from './auth.client';
import { AbstractNavigator } from './abstract-navigator';
import { Location } from '@angular/common';
import { AuthClientConfig } from './auth.config';

@Injectable({
providedIn: 'root',
Expand Down Expand Up @@ -88,6 +89,7 @@ export class AuthService implements OnDestroy {

constructor(
@Inject(Auth0ClientService) private auth0Client: Auth0Client,
private configFactory: AuthClientConfig,
private location: Location,
private navigator: AbstractNavigator
) {
Expand Down Expand Up @@ -253,11 +255,13 @@ export class AuthService implements OnDestroy {

private shouldHandleCallback(): Observable<boolean> {
return of(this.location.path()).pipe(
map(
(search) =>
map((search) => {
return (
(search.includes('code=') || search.includes('error=')) &&
search.includes('state=')
)
search.includes('state=') &&
!this.configFactory.get()?.skipRedirectCallback
);
})
);
}

Expand Down
1 change: 1 addition & 0 deletions projects/playground/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
const authConfig: AuthConfig = {
clientId: 'wLSIP47wM39wKdDmOj6Zb5eSEw3JVhVp',
domain: 'brucke.auth0.com',
skipRedirectCallback: window.location.pathname === '/other-callback',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to have a route set up to handle this? How does this work in the playground from a user's perspective?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you want to use this, yes you do.
I added it to the playground as a matter of showing how to configure it, but we do not realy integrate our playground with another OAuth provider tho.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case it might be better to instead show a usage example in the docs against the new property - I can imagine people will look there first before they dive into the playground code.

Do you think this also merits a section in the readme about usage with other providers?

Copy link
Member Author

@frederikprijck frederikprijck Nov 18, 2020

Choose a reason for hiding this comment

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

Yes I think that is a good idea and I updated the PR.

API Docs:
image

README:
image

};

/**
Expand Down