Skip to content

Commit

Permalink
#333 oauth2-spike improve error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
vmarc committed Dec 23, 2023
1 parent 91efd22 commit dcd5cb7
Show file tree
Hide file tree
Showing 14 changed files with 114 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ class AuthenticationController(oauthClientId: String, crypto: Crypto, cryptoKey:
@GetMapping(value = Array("/api/logout"))
@ResponseBody def logout(response: HttpServletResponse): ResponseEntity[_] = {
log.info("GET logout()")
// TODO accessToken from cookie, and then issue revoke on openstreetmap.org
response.addCookie(createCookie("", 0))
response.addCookie(createCookie("", 0)) // erase cookie on browser
new ResponseEntity[String]("", HttpStatus.OK)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package kpn.backend
import org.springframework.boot.SpringApplication
import org.springframework.boot.autoconfigure.SpringBootApplication


object BackendApplication {
def main(args: Array[String]): Unit = {
val app: Array[Class[_]] = Array(classOf[BackendApplication])
Expand Down
2 changes: 1 addition & 1 deletion oauth2-spike/frontend/src/app/app.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { provideRouter } from '@angular/router';
import { provideOAuthClient } from 'angular-oauth2-oidc';

import { routes } from './app.routes';
import { UserService } from './service/user.service';
import { UserService } from './user/user.service';

export const appConfig: ApplicationConfig = {
providers: [
Expand Down
12 changes: 6 additions & 6 deletions oauth2-spike/frontend/src/app/app.routes.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Routes } from '@angular/router';
import { AuthenticatedComponent } from './pages/authenticated.component';
import { UserAuthenticatedComponent } from './user/user-authenticated.component';
import { HomeComponent } from './pages/home.component';
import { LoginComponent } from './pages/login.component';
import { LogoutComponent } from './pages/logout.component';
import { UserLoginComponent } from './user/user-login.component';
import { UserLogoutComponent } from './user/user-logout.component';
import { Page1Component } from './pages/page1.component';
import { Page2Component } from './pages/page2.component';
import { Page3Component } from './pages/page3.component';
Expand All @@ -22,15 +22,15 @@ export const routes: Routes = [
},
{
path: 'login',
component: LoginComponent,
component: UserLoginComponent,
},
{
path: 'logout',
component: LogoutComponent,
component: UserLogoutComponent,
},
{
path: 'authenticated',
component: AuthenticatedComponent,
component: UserAuthenticatedComponent,
},
{
path: '',
Expand Down
49 changes: 0 additions & 49 deletions oauth2-spike/frontend/src/app/pages/login.component.ts

This file was deleted.

14 changes: 7 additions & 7 deletions oauth2-spike/frontend/src/app/pages/page.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { Input } from '@angular/core';
import { Component } from '@angular/core';
import { RouterOutlet } from '@angular/router';
import { RouterLink } from '@angular/router';
import { UserService } from '../service/user.service';
import { UserStore } from '../service/user.store';
import { LinkLoginComponent } from './link-login.component';
import { LinkLogoutComponent } from './link-logout.component';
import { UserService } from '../user/user.service';
import { UserStore } from '../user/user.store';
import { UserLinkLoginComponent } from '../user/user-link-login.component';
import { UserLinkLogoutComponent } from '../user/user-link-logout.component';

@Component({
selector: 'kpn-page',
Expand All @@ -25,13 +25,13 @@ import { LinkLogoutComponent } from './link-logout.component';
@if (user(); as name) {
<p>Logged in as: {{ name }}</p>
<kpn-link-logout />
<kpn-user-link-logout />
} @else {
<p>Not logged in</p>
<kpn-link-login />
<kpn-user-link-login />
}
`,
imports: [LinkLoginComponent, RouterLink, RouterOutlet, LinkLogoutComponent],
imports: [RouterLink, RouterOutlet, UserLinkLoginComponent, UserLinkLogoutComponent],
})
export class PageComponent {
private readonly userService = inject(UserService);
Expand Down
21 changes: 21 additions & 0 deletions oauth2-spike/frontend/src/app/user/user-authenticated.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { inject } from '@angular/core';
import { Component } from '@angular/core';
import { UserErrorComponent } from './user-error.component';
import { UserService } from './user.service';

@Component({
selector: 'kpn-user-authenticated',
standalone: true,
template: `
<p>logging in...</p>
<kpn-user-error />
`,
imports: [UserErrorComponent],
})
export class UserAuthenticatedComponent {
private readonly userService = inject(UserService);

constructor() {
this.userService.authenticated();
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { OnDestroy } from '@angular/core';
import { inject } from '@angular/core';
import { Component } from '@angular/core';
import { UserService } from '../service/user.service';
import { UserStore } from '../service/user.store';
import { UserStore } from './user.store';

@Component({
selector: 'kpn-authenticated',
selector: 'kpn-user-error',
standalone: true,
template: `
<p>logging in...</p>
@if (error(); as message) {
<p class="error">
{{ message }}
Expand All @@ -20,13 +19,12 @@ import { UserStore } from '../service/user.store';
}
`,
})
export class AuthenticatedComponent {
private readonly userService = inject(UserService);
export class UserErrorComponent implements OnDestroy {
private readonly userStore = inject(UserStore);
readonly error = this.userStore.error;
readonly errorDetail = this.userStore.errorDetail;

constructor() {
this.userService.authenticated();
ngOnDestroy() {
this.userStore.resetError();
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { inject } from '@angular/core';
import { ChangeDetectionStrategy } from '@angular/core';
import { Component } from '@angular/core';
import { UserService } from '../service/user.service';
import { UserService } from './user.service';

@Component({
selector: 'kpn-link-login',
selector: 'kpn-user-link-login',
changeDetection: ChangeDetectionStrategy.OnPush,
template: ` <a rel="nofollow noreferrer" (click)="login()">login</a> `,
standalone: true,
})
export class LinkLoginComponent {
export class UserLinkLoginComponent {
private readonly userService = inject(UserService);

login(): void {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { inject } from '@angular/core';
import { ChangeDetectionStrategy } from '@angular/core';
import { Component } from '@angular/core';
import { UserService } from '../service/user.service';
import { UserService } from './user.service';

@Component({
selector: 'kpn-link-logout',
selector: 'kpn-user-link-logout',
changeDetection: ChangeDetectionStrategy.OnPush,
template: ` <a rel="nofollow noreferrer" (click)="logout()">logout</a> `,
standalone: true,
})
export class LinkLogoutComponent {
export class UserLinkLogoutComponent {
private readonly userService = inject(UserService);

logout(): void {
Expand Down
32 changes: 32 additions & 0 deletions oauth2-spike/frontend/src/app/user/user-login.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { inject } from '@angular/core';
import { Component } from '@angular/core';
import { UserService } from './user.service';
import { UserErrorComponent } from './user-error.component';

@Component({
selector: 'kpn-user-login',
standalone: true,
template: `
<h1>Login</h1>
<p>Here goes some explanation about the login process.</p>
<p>Click the login button to login via the OpenStreetMap website.</p>
<p>Click the cancel link to return to the previous page.</p>
<div class="menu top-spacer">
<button (click)="login()">login</button>
<a (click)="cancel()">cancel</a>
</div>
<kpn-user-error />
`,
imports: [UserErrorComponent],
})
export class UserLoginComponent {
private readonly userService = inject(UserService);

login(): void {
this.userService.login();
}

cancel(): void {
this.userService.navigateToReturnUrl();
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { inject } from '@angular/core';
import { Component } from '@angular/core';
import { UserService } from '../service/user.service';
import { UserService } from './user.service';
import { UserErrorComponent } from './user-error.component';

@Component({
selector: 'kpn-logout',
selector: 'kpn-user-logout',
standalone: true,
template: `
<h1>Logout</h1>
Expand All @@ -12,9 +13,11 @@ import { UserService } from '../service/user.service';
<button (click)="logout()">logout</button>
<a (click)="cancel()">cancel</a>
</div>
<kpn-user-error />
`,
imports: [UserErrorComponent],
})
export class LogoutComponent {
export class UserLogoutComponent {
private readonly userService = inject(UserService);

logout(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import { catchError } from 'rxjs';
import { map } from 'rxjs';
import { Observable } from 'rxjs';
import { tap } from 'rxjs';
import { BrowserStorageService } from './browser-storage.service';
import { Subscriptions } from './subscriptions';
import { BrowserStorageService } from '../service/browser-storage.service';
import { Subscriptions } from '../service/subscriptions';
import { UserStore } from './user.store';

interface AuthorizationError {
Expand Down Expand Up @@ -69,8 +69,7 @@ export class UserService implements OnDestroy {
}

login(): void {
this.userStore.updateError(null);
this.userStore.updateErrorDetail(null);
this.userStore.resetError();
this.loadDiscoveryDocumentAndTryLogin().subscribe(() => {
if (this.returnUrl) {
this.debug(`login() initCodeFlow() returnUrl=${this.returnUrl}`);
Expand All @@ -86,25 +85,31 @@ export class UserService implements OnDestroy {
logout(): void {
this.http
.get('/api/logout', { responseType: 'text' }) // erase the cookie
.pipe(
// TODO catchError !!!
tap(() => {
this.updateUser(null);
if (this.returnUrl) {
const url = this.returnUrl;
this.returnUrl = null;
this.debug(`navigateByUrl(${url})`);
this.router.navigateByUrl(url);
}
})
)
.subscribe({
next: (v) => this.debug('logout() next', v),
error: (e) => this.debug('logout() error', e),
complete: () => this.debug('logout() completed.'),
next: () => this.logoutUser(),
error: (error) => this.logoutError(error),
});
}

private logoutUser(): void {
this.updateUser(null);
if (this.returnUrl) {
const url = this.returnUrl;
this.returnUrl = null;
this.debug(`navigateByUrl(${url})`);
this.router.navigateByUrl(url);
}
}

private logoutError(error: any): void {
if (error instanceof HttpErrorResponse) {
const httpErrorResponse = error as HttpErrorResponse;
this.userStore.updateError('Could not logout');
this.userStore.updateErrorDetail(httpErrorResponse.message);
}
this.debug('logout() error', error);
}

loginLinkClicked(): void {
this.returnUrl = this.router.url; // preserve url to return to after login
this.router.navigate(['/login']);
Expand Down Expand Up @@ -252,11 +257,7 @@ export class UserService implements OnDestroy {
}
})
)
.subscribe({
next: (v) => this.debug('/api/authenticated HTTP response', v),
error: (e) => this.debug('/api/authenticated HTTP Error', e),
complete: () => this.debug('/api/authenticated HTTP request completed.'),
});
.subscribe();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ export const UserStore = signalStore(
updateErrorDetail: (errorDetail: string | null) => {
patchState(state, { errorDetail });
},
resetError: () => {
patchState(state, { error: null, errorDetail: null });
},
};
}),
withHooks({
Expand Down

0 comments on commit dcd5cb7

Please sign in to comment.