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

SHIELD-71 SHIELD-78 Modified source of id_token. #7145

Merged
merged 22 commits into from
Jun 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/automate-dex/habitat/templates/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ staticClients:
- id: automate-session
secret: secretchangeme # TODO
redirectURIs:
- /session/callback
- /signin
name: Automate Session Service
{{- if cfg.connectors}}
connectors:
Expand Down
4 changes: 3 additions & 1 deletion components/automate-ui/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ import { DataFeedConfigDetailsComponent } from './pages/data-feed-config-details
import {
DataFeedTableComponent
} from './page-components/data-feed-table/data-feed-table.component';
import { SigninService } from './services/signin/signin.service';



Expand Down Expand Up @@ -395,7 +396,8 @@ import {
TelemetryService,
UserPermsRequests,
UserPreferencesRequests,
UserRequests
UserRequests,
SigninService
],
bootstrap: [ AppComponent ],
schemas: [ CUSTOM_ELEMENTS_SCHEMA ]
Expand Down
124 changes: 48 additions & 76 deletions components/automate-ui/src/app/pages/signin/signin.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,49 +6,43 @@ import { MockChefSessionService } from 'app/testing/mock-chef-session.service';
import { SigninComponent } from './signin.component';
import { MockComponent } from 'ng2-mock-component';
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { ReplaySubject } from 'rxjs';

class MockActivatedRoute {
private subject = new ReplaySubject<string>();

constructor(initialFragment: string = '') {
this.setFragment(initialFragment);
}

readonly fragment = this.subject.asObservable();
readonly snapshot = {};

setFragment(frag: string) {
this.subject.next(frag);
}
}
function setCookie(key: string, value: string): void {
document.cookie = `${key}=${value}; Path=/;`;
import { Observable, of } from 'rxjs';
import { CallbackResponse, SigninService } from 'app/services/signin/signin.service';
import { HttpClientTestingModule } from '@angular/common/http/testing';

class ActivatedRouteMock {
queryParams = new Observable(observer => {
const urlParams = {
param1: 'some',
param2: 'params'
};
observer.next(urlParams);
observer.complete();
});
}

function deleteIdTokenFromCookie(): void {
// Expire id_token cookie once it's set in localStorage
document.cookie = `id_token=; Path=/; Expires=${new Date().toUTCString()};`;
class SignIn {
callback() {
const data: CallbackResponse = {
state: 'test',
id_token: 'test'
};
return of(data);
}
}

describe('SigninComponent', () => {
let component: SigninComponent;
let fixture: ComponentFixture<SigninComponent>;
let chefSessionService: ChefSessionService;
let activatedRoute: MockActivatedRoute;

const valid_id_token_key = 'id_token';
// tslint:disable-next-line:max-line-length
const valid_id_token = 'eyJhbGciOiJSUzI1NiIsImtpZCI6ImJkOWY5ZGFkZDc4ZDEyOWFlN2I2ODZhZTU0NjJhOWYzY2JmMDY1MTUifQ.eyJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjQyMDAvZGV4Iiwic3ViIjoiQ2cwd0xUTTROUzB5T0RBNE9TMHdFZ1J0YjJOciIsImF1ZCI6ImF1dG9tYXRlLXVpIiwiZXhwIjoxNTA5NzIwMTgzLCJpYXQiOjE1MDk2MzM3ODMsImF0X2hhc2giOiJ4ck1fTXNmLUd1dmY1dzRGeWY1THVRIiwiZW1haWwiOiJraWxnb3JlQGtpbGdvcmUudHJvdXQiLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiZ3JvdXBzIjpbImF1dGhvcnMiXSwibmFtZSI6IktpbGdvcmUgVHJvdXQifQ.CsBjk47MdwpkneBsbc9NEIx8TskokPDrd3Bp-C4GhcdC-eZH-vOKBnRytMi7_GcOchevo7KCmwjzZllC-AgJMd7b5SBWVjDzLQuS8D9zIX_t_vf3c_wwl4R_fYjBiO7wmm3u-VQGCmxX4UjqyfzWCT-FYwLH5WctVusM3bdlAF0FiLndkmiyAaNFbxMznlDwmrys39in4oV9srxZnXrK-ydlhpJJzETrwBVmAhDzKJO62GC6WcFQYFeQ0Dtb6eBSFaRBi7LmM5TUT_qcIW-LRGcfa7h2DfifKEgCFuv6QjUXb8B7fxRZNMQyAcoVV9qZK8Nd51l-anDD1PI4J12hyw';


beforeEach(() => {
activatedRoute = new MockActivatedRoute('');
});

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [
HttpClientTestingModule,
RouterTestingModule.withRoutes([{ path: '**', component: SigninComponent }]) // match all
],
declarations: [
Expand All @@ -57,8 +51,9 @@ describe('SigninComponent', () => {
],
providers: [
// Note: the [routerLink] in the template requires snapshot to exist
{ provide: ActivatedRoute, useValue: activatedRoute },
{ provide: ChefSessionService, useClass: MockChefSessionService }
{ provide: ActivatedRoute, useClass: ActivatedRouteMock},
{ provide: ChefSessionService, useClass: MockChefSessionService },
{ provide: SigninService, userClass: SignIn}
],
schemas: [ CUSTOM_ELEMENTS_SCHEMA ]
})
Expand All @@ -77,60 +72,54 @@ describe('SigninComponent', () => {

describe('ngOnInit', () => {
describe('sets error', () => {
it('to true when fragment is missing', () => {
activatedRoute.setFragment(null);
fixture.detectChanges();

expect((component as any).error).toEqual(true);
});

it('to true when id_token does not exist in cookie', () => {
const [id_token, state] = component.idTokenAndStateFromCookieAndFragment('');
deleteIdTokenFromCookie();
it('to true when id_token does not exist', () => {
const err = component.setIdAndPath('', '');
fixture.detectChanges();
expect(id_token).toEqual(null);
expect(state).toEqual(null);
const id_token = (component as any).idToken;
const path = (component as any).path;
expect(id_token).toEqual('');
expect(path).toEqual('');

expect((component as any).error).toEqual(true);
expect(err).toEqual(true);
});

it('to false when id_token exists in cookie', () => {
setCookie(valid_id_token_key, valid_id_token);
const [id_token, state] = component.idTokenAndStateFromCookieAndFragment('state=');
it('to false when id_token exists', () => {
const err = component.setIdAndPath(valid_id_token, '');
fixture.detectChanges();
const id_token = (component as any).idToken;
const path = (component as any).path;
expect(id_token).toBe(valid_id_token);
expect(state).toBe('');
expect(path).toBe('');

expect((component as any).error).toEqual(false);
expect(err).toEqual(false);
});

it('to true when id token from cookie cannot be decoded', () => {
it('to true when id token cannot be decoded', () => {
const not_a_jwt_value = 'NOTAJWT';
setCookie(valid_id_token_key, not_a_jwt_value);
fixture.detectChanges();
const [id_token, state] = component.idTokenAndStateFromCookieAndFragment('state=');
const error = component.setIdAndPath(not_a_jwt_value, '');
fixture.detectChanges();
const id_token = (component as any).idToken;
const path = (component as any).path;
expect(id_token).toBe(not_a_jwt_value);
expect(state).toBe('');

expect((component as any).error).toEqual(true);
expect(path).toBe('');
expect(error).toEqual(true);
});
});

it('stores information on success', () => {
activatedRoute.setFragment('state=foo');
setCookie(valid_id_token_key, valid_id_token);
fixture.detectChanges();
const [id_token, state] = component.idTokenAndStateFromCookieAndFragment('state=foo');
component.setIdAndPath(valid_id_token, 'foo');
fixture.detectChanges();
const id_token = (component as any).idToken;
const path = (component as any).path;

const id = (component as any).id;
expect(id.sub).toEqual('Cg0wLTM4NS0yODA4OS0wEgRtb2Nr');
expect(id.name).toEqual('Kilgore Trout');
expect(id.email).toEqual('kilgore@kilgore.trout');
expect(id.groups).toEqual(['authors']);
expect((component as any).idToken).toEqual(id_token);
expect(state).toEqual('foo');
expect(path).toEqual('foo');
});
});

Expand Down Expand Up @@ -205,21 +194,4 @@ describe('SigninComponent', () => {
});
});

describe('#idTokenFromCookieAndstateFromFragment', () => {
// empty state: not special, but common
it('returns empty id_token and matches an empty state', () => {
const actual = component.idTokenAndStateFromCookieAndFragment('state=');
expect(actual).toEqual([null, '']);
});

it('if the match fails returns nothing', () => {
const actual = component.idTokenAndStateFromCookieAndFragment('notwhatweexpect');
expect(actual).toEqual([null, null]);
});

it('expects the ID token to be non-empty', () => {
const actual = component.idTokenAndStateFromCookieAndFragment('state=%2Fnodes');
expect(actual).toEqual([null, '%2Fnodes']);
});
});
});
85 changes: 47 additions & 38 deletions components/automate-ui/src/app/pages/signin/signin.component.ts
Original file line number Diff line number Diff line change
@@ -1,57 +1,75 @@
import { Component, OnInit } from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';
import { Component, OnDestroy, OnInit } from '@angular/core';
import { ActivatedRoute, Params, Router } from '@angular/router';
import { ChefSessionService } from 'app/services/chef-session/chef-session.service';
import { IDToken, Jwt } from 'app/helpers/jwt/jwt';
import { ReplaySubject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
import { SigninService } from 'app/services/signin/signin.service';


@Component({
selector: 'app-signin',
templateUrl: './signin.component.html',
styleUrls: ['./signin.component.scss']
})
export class SigninComponent implements OnInit {
export class SigninComponent implements OnInit, OnDestroy {
public error = false;
public path: string; // from OIDC redirect, but sanitized
private idToken: string;
private id: IDToken;
private destroyed$: ReplaySubject<boolean> = new ReplaySubject(1);
private searchParams: Params;

constructor(
private route: ActivatedRoute,
private router: Router,
private session: ChefSessionService
) { }
private session: ChefSessionService,
private signinService: SigninService,
private route: ActivatedRoute
) {
this.route.queryParams
.pipe(takeUntil(this.destroyed$))
.subscribe(params => {
this.searchParams = params;
});
}

ngOnInit() {
// Reminder: URL fragment has to be treated as user-provided input, and can
// NOT be trusted in any way. /!\
this.route.fragment.subscribe((fragment: string) => {
if (fragment === null) {
this.error = true;
return;
}
let state: string;
[this.idToken, state] = this.idTokenAndStateFromCookieAndFragment(fragment);
if (this.idToken === null) {
this.error = true;
return;
}
this.id = Jwt.parseIDToken(this.idToken);
if (this.id === null) {
this.error = true;
this.signinService.callback(this.searchParams)
.pipe(takeUntil(this.destroyed$))
.subscribe((res) => {
this.error = this.setIdAndPath(res.id_token, res.state);
if (this.error) {
return;
}
this.path = this.pathFromState(state);

this.error = false;
this.setSession();
localStorage.setItem('manual-upgrade-banner', 'true');
this.deleteIdTokenFromCookie(this.idToken);
this.router.navigateByUrl(this.path);
}, (err) => {
if (err.status === 200 && 'url' in err) {
window.location.href = err.url;
return;
}
this.error = true;
});
}

deleteIdTokenFromCookie(token: string): void {
// Expire id_token cookie once it's set in localStorage
document.cookie = `id_token=${token}; Path=/; Expires=${new Date().toUTCString()};`;
ngOnDestroy() {
this.destroyed$.next(true);
this.destroyed$.complete();
}

setIdAndPath(idToken: string, state: string): boolean {
let error = false;
this.idToken = idToken;
this.path = this.pathFromState(state);
if (this.idToken === '') {
error = true;
}
this.id = Jwt.parseIDToken(this.idToken);
if (this.id === null) {
error = true;
}
return error;
}

setSession(): void {
Expand Down Expand Up @@ -82,13 +100,4 @@ export class SigninComponent implements OnInit {
}
return path;
}

idTokenAndStateFromCookieAndFragment(fragment: string): [string | null, string | null] {
// Note: we only get an ID token and state now, so we match from ^ to $
const state_match = fragment.match('^state=([^&]*)$');
const id_token_match = `; ${document.cookie}`.match(';\\s*id_token=([^;]+)');
const id_token = id_token_match ? id_token_match[1] : null;
const state = state_match ? state_match[1] : null;
return [id_token, state];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export class HttpClientAuthInterceptor implements HttpInterceptor {
}

intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
if (request.headers.get('skip')) {
return next.handle(request);
}
let headers = request.headers;

// TODO(sr): Sadly, our UI code depends on the API ignoring unknown fields in the
Expand Down
20 changes: 20 additions & 0 deletions components/automate-ui/src/app/services/signin/signin.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Injectable } from '@angular/core';
import { HttpClient } from '@angular/common/http';
import { Observable } from 'rxjs';
import { Params } from '@angular/router';

const callbackPath = '/session/callback';
export interface CallbackResponse {
state: string;
id_token: string;
}

@Injectable()
export class SigninService {

constructor(private httpClient: HttpClient) {}

callback(params: Params): Observable<CallbackResponse> {
return this.httpClient.get<any>(callbackPath, {headers: {skip: 'true'}, params});
}
}
6 changes: 4 additions & 2 deletions components/session-service/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,17 @@ It's effective in the callback handler below.

### GET /callback

OIDC callback handler; exchanges `code`, stores `refresh_token`, creates a session, redirects
with `id_token` in URL fragment.
Dex redirects to /signin route in browser which in turn calls the /callback endpoint.
OIDC callback handler; exchanges `code`, stores `refresh_token`, creates a session, sends
`id_token` as a json response.

Diagram for /new -> /callback -> browser has session flow:

![](./docs/diagrams/new_session.png)

#### `redirect_uri` provided

Dex redirects to /signin route in browser which in turn calls the /callback endpoint.
If the session data carries a `redirect_uri`, the callback handler slightly changes its
behavior: it will generate a new `code`, associate the exchanged `id_token` with it, and
redirect the browser to the Builder Signin URL as configured with session-service.
Expand Down
Binary file modified components/session-service/docs/diagrams/new_session.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading