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

* SHIELD-64 Added Secure in id_token cookie.

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* temp commit

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* working signin removed redirect

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* removed cookie for signin

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* removed cookie for signin

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* removed cookie for signin

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* Removed redundant jump.

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* improvements in observable handling

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* improvements in observable handling

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* fixed bldr login

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* Modified unit test for ui

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* Added CallbackService in signin spec.

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* Injected CallbackService for testcases.

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* fixed lint in ui

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* Renamed CallbackService to SigninService

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* Fixed seesion-service unit tests

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

* fixed unit test

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* fixed unit test

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* fixed unit test

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* updated some diagrams and docs

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* updated diagram

Signed-off-by: Vivek Shankar <vshankar@progress.com>

* removed unused variable.

Signed-off-by: Atul Krishna <Atul.Krishna@progress.com>

Co-authored-by: Vivek Shankar <vshankar@progress.com>

components/automate-dex/habitat/templates/config.yml

@@ -56,7 +56,7 @@ staticClients:
   - id: automate-session
     secret: secretchangeme # TODO
     redirectURIs:
-      - /session/callback
+      - /signin
     name: Automate Session Service
 {{- if cfg.connectors}}
 connectors:

components/automate-ui/src/app/app.module.ts

@@ -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';
 
 
 
@@ -395,7 +396,8 @@ import {
     TelemetryService,
     UserPermsRequests,
     UserPreferencesRequests,
-    UserRequests
+    UserRequests,
+    SigninService
   ],
   bootstrap: [ AppComponent ],
   schemas: [ CUSTOM_ELEMENTS_SCHEMA ]

components/automate-ui/src/app/pages/signin/signin.component.spec.ts

@@ -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: [
@@ -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 ]
     })
@@ -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');
     });
   });
 
@@ -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']);
-    });
-  });
 });

components/automate-ui/src/app/pages/signin/signin.component.ts

@@ -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 {
@@ -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];
-  }
 }

components/automate-ui/src/app/services/http/http-client-auth.interceptor.ts

@@ -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

components/automate-ui/src/app/services/signin/signin.service.ts

@@ -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});
+  }
+}

components/session-service/README.md

@@ -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.