From 5301dc8caf38b03e54de02cdc6edf57b97fbd807 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Fri, 7 Feb 2025 03:01:23 -0500 Subject: [PATCH 1/3] implement github/oauth2 authentication --- .../Api/Auth/AuthController.cs | 129 +++++++++--------- .../GitHubAuthenticationHandler.cs | 22 ++- rubberduckvba.Server/Program.cs | 4 +- rubberduckvba.client/src/app/app.module.ts | 12 +- .../auth-menu/auth-menu.component.html | 98 +++++++++++++ .../auth-menu/auth-menu.component.ts | 79 +++++++++++ .../loading-content.component.html | 2 +- .../loading-content.component.ts | 1 + .../nav-menu/nav-menu.component.html | 127 ++++++++--------- .../components/nav-menu/nav-menu.component.ts | 1 + .../src/app/model/feature.model.ts | 6 + .../src/app/routes/auth/auth.component.html | 1 + .../src/app/routes/auth/auth.component.ts | 16 +++ .../src/app/services/api-client.service.ts | 25 +++- .../src/app/services/auth.service.ts | 57 ++++++++ .../src/app/services/data.service.ts | 63 +++++++-- 16 files changed, 489 insertions(+), 154 deletions(-) create mode 100644 rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html create mode 100644 rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts create mode 100644 rubberduckvba.client/src/app/routes/auth/auth.component.html create mode 100644 rubberduckvba.client/src/app/routes/auth/auth.component.ts create mode 100644 rubberduckvba.client/src/app/services/auth.service.ts diff --git a/rubberduckvba.Server/Api/Auth/AuthController.cs b/rubberduckvba.Server/Api/Auth/AuthController.cs index 10c8c3d..c0965d7 100644 --- a/rubberduckvba.Server/Api/Auth/AuthController.cs +++ b/rubberduckvba.Server/Api/Auth/AuthController.cs @@ -1,30 +1,32 @@ -using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; using Octokit; using Octokit.Internal; using System.Security.Claims; -using System.Text; namespace rubberduckvba.Server.Api.Auth; public record class UserViewModel { - public static UserViewModel Anonymous { get; } = new UserViewModel { Name = "(anonymous)", HasOrgRole = false }; + public static UserViewModel Anonymous { get; } = new UserViewModel { Name = "(anonymous)", IsAuthenticated = false, IsAdmin = false }; public string Name { get; init; } = default!; - public bool HasOrgRole { get; init; } + public bool IsAuthenticated { get; init; } + public bool IsAdmin { get; init; } } - +public record class SignInViewModel +{ + public string? State { get; init; } + public string? Code { get; init; } + public string? Token { get; init; } +} [ApiController] -[AllowAnonymous] -public class AuthController(IOptions configuration, IOptions api) : ControllerBase +public class AuthController(IOptions configuration, IOptions api, ILogger logger) : ControllerBase { [HttpGet("auth")] - [AllowAnonymous] - public ActionResult Index() + public IActionResult Index() { var claims = HttpContext.User.Claims.ToDictionary(claim => claim.Type, claim => claim.Value); var hasName = claims.TryGetValue(ClaimTypes.Name, out var name); @@ -37,10 +39,12 @@ public ActionResult Index() return BadRequest(); } + var isAuthenticated = HttpContext.User.Identity?.IsAuthenticated ?? false; var model = new UserViewModel { Name = name, - HasOrgRole = (HttpContext.User.Identity?.IsAuthenticated ?? false) && role == configuration.Value.OwnerOrg + IsAuthenticated = isAuthenticated, + IsAdmin = isAuthenticated && role == configuration.Value.OwnerOrg }; return Ok(model); @@ -52,12 +56,13 @@ public ActionResult Index() } [HttpPost("auth/signin")] - [AllowAnonymous] - public async Task SignIn() + public IActionResult SessionSignIn(SignInViewModel vm) { - var xsrf = Guid.NewGuid().ToString(); - HttpContext.Session.SetString("xsrf:state", xsrf); - await HttpContext.Session.CommitAsync(); + if (User.Identity?.IsAuthenticated ?? false) + { + logger.LogInformation("Signin was requested, but user is already authenticated. Redirecting to home page..."); + return Redirect("/"); + } var clientId = configuration.Value.ClientId; var agent = configuration.Value.UserAgent; @@ -67,53 +72,45 @@ public async Task SignIn() { AllowSignup = false, Scopes = { "read:user", "read:org" }, - State = xsrf + State = vm.State }; + logger.LogInformation("Requesting OAuth app GitHub login url..."); var url = github.Oauth.GetGitHubLoginUrl(request); if (url is null) { + logger.LogInformation("OAuth login was cancelled by the user or did not return a url."); return Forbid(); } - // TODO log url - //return Redirect(url.ToString()); - return RedirectToAction("Index", "Home"); + logger.LogInformation("Returning the login url for the client to redirect. State: {xsrf}", vm.State); + return Ok(url.ToString()); } - [HttpGet("auth/github")] - [AllowAnonymous] - public async Task GitHubCallback(string code, string state) + [HttpPost("auth/github")] + public async Task OnGitHubCallback(SignInViewModel vm) { - if (string.IsNullOrWhiteSpace(code)) - { - return BadRequest(); - } - - var expected = HttpContext.Session.GetString("xsrf:state"); - HttpContext.Session.Clear(); - await HttpContext.Session.CommitAsync(); - - if (state != expected) - { - return BadRequest(); - } - + logger.LogInformation("OAuth token was received. State: {state}", vm.State); var clientId = configuration.Value.ClientId; var clientSecret = configuration.Value.ClientSecret; var agent = configuration.Value.UserAgent; var github = new GitHubClient(new ProductHeaderValue(agent)); - var request = new OauthTokenRequest(clientId, clientSecret, code); + var request = new OauthTokenRequest(clientId, clientSecret, vm.Code); var token = await github.Oauth.CreateAccessToken(request); + if (token is null) + { + logger.LogWarning("OAuth access token was not created."); + return Unauthorized(); + } - await AuthorizeAsync(token.AccessToken); - - return Ok(); + logger.LogInformation("OAuth access token was created. Authorizing..."); + var authorizedToken = await AuthorizeAsync(token.AccessToken); + return authorizedToken is null ? Unauthorized() : Ok(vm with { Token = authorizedToken }); } - private async Task AuthorizeAsync(string token) + private async Task AuthorizeAsync(string token) { try { @@ -122,42 +119,44 @@ private async Task AuthorizeAsync(string token) var githubUser = await github.User.Current(); if (githubUser.Suspended) { - throw new InvalidOperationException("User is suspended"); + logger.LogWarning("User {name} with login '{login}' ({url}) is a suspended GitHub account and will not be authorized.", githubUser.Name, githubUser.Login, githubUser.Url); + return default; } - var emailClaim = new Claim(ClaimTypes.Email, githubUser.Email); - var identity = new ClaimsIdentity("github", ClaimTypes.Name, ClaimTypes.Role); - if (identity != null) - { - identity.AddClaim(new Claim(ClaimTypes.Name, githubUser.Login)); - - var orgs = await github.Organization.GetAllForUser(githubUser.Login); - var rdOrg = orgs.SingleOrDefault(org => org.Id == configuration.Value.RubberduckOrgId); + identity.AddClaim(new Claim(ClaimTypes.Name, githubUser.Login)); + logger.LogInformation("Creating claims identity for GitHub login '{login}'...", githubUser.Login); - if (rdOrg != null) - { - identity.AddClaim(new Claim(ClaimTypes.Role, configuration.Value.OwnerOrg)); - identity.AddClaim(new Claim(ClaimTypes.Authentication, token)); - identity.AddClaim(new Claim("access_token", token)); + var orgs = await github.Organization.GetAllForUser(githubUser.Login); + var rdOrg = orgs.SingleOrDefault(org => org.Id == configuration.Value.RubberduckOrgId); - var principal = new ClaimsPrincipal(identity); + if (rdOrg != null) + { + identity.AddClaim(new Claim(ClaimTypes.Role, configuration.Value.OwnerOrg)); + identity.AddClaim(new Claim(ClaimTypes.Authentication, token)); + identity.AddClaim(new Claim("access_token", token)); + logger.LogDebug("GitHub Organization claims were granted. Creating claims principal..."); - var issued = DateTime.UtcNow; - var expires = issued.Add(TimeSpan.FromMinutes(50)); - var roles = string.Join(",", identity.Claims.Where(claim => claim.Type == ClaimTypes.Role).Select(claim => claim.Value)); + var principal = new ClaimsPrincipal(identity); + var roles = string.Join(",", identity.Claims.Where(claim => claim.Type == ClaimTypes.Role).Select(claim => claim.Value)); - HttpContext.User = principal; - Thread.CurrentPrincipal = HttpContext.User; + HttpContext.User = principal; + Thread.CurrentPrincipal = HttpContext.User; - var jwt = principal.AsJWT(api.Value.SymetricKey, configuration.Value.JwtIssuer, configuration.Value.JwtAudience); - HttpContext.Session.SetString("jwt", jwt); - } + logger.LogInformation("GitHub user with login {login} has signed in with role authorizations '{role}'.", githubUser.Login, configuration.Value.OwnerOrg); + return token; + } + else + { + logger.LogWarning("User {name} ({email}) with login '{login}' is not a member of organization ID {org} and will not be authorized.", githubUser.Name, githubUser.Email, githubUser.Login, configuration.Value.RubberduckOrgId); + return default; } } catch (Exception) { // just ignore: configuration needs the org (prod) client app id to avoid throwing this exception + logger.LogWarning("An exception was thrown. Verify GitHub:ClientId and GitHub:ClientSecret configuration; authorization fails."); + return default; } } } diff --git a/rubberduckvba.Server/GitHubAuthenticationHandler.cs b/rubberduckvba.Server/GitHubAuthenticationHandler.cs index 1ed3f61..36dd5ad 100644 --- a/rubberduckvba.Server/GitHubAuthenticationHandler.cs +++ b/rubberduckvba.Server/GitHubAuthenticationHandler.cs @@ -20,15 +20,23 @@ public GitHubAuthenticationHandler(IGitHubClientService github, protected async override Task HandleAuthenticateAsync() { - var token = Context.Request.Headers["X-ACCESS-TOKEN"].SingleOrDefault(); - if (token is null) + try { + var token = Context.Request.Headers["X-ACCESS-TOKEN"].SingleOrDefault(); + if (token is null) + { + return AuthenticateResult.NoResult(); + } + + var principal = await _github.ValidateTokenAsync(token); + return principal is ClaimsPrincipal + ? AuthenticateResult.Success(new AuthenticationTicket(principal, "github")) + : AuthenticateResult.NoResult(); + } + catch (InvalidOperationException e) + { + Logger.LogError(e, e.Message); return AuthenticateResult.NoResult(); } - - var principal = await _github.ValidateTokenAsync(token); - return principal is ClaimsPrincipal - ? AuthenticateResult.Success(new AuthenticationTicket(principal, "github")) - : AuthenticateResult.NoResult(); } } diff --git a/rubberduckvba.Server/Program.cs b/rubberduckvba.Server/Program.cs index 0f1cd38..425a0e0 100644 --- a/rubberduckvba.Server/Program.cs +++ b/rubberduckvba.Server/Program.cs @@ -90,6 +90,7 @@ public static void Main(string[] args) app.UseHttpsRedirection(); app.UseRouting(); + app.UseSession(); app.UseAuthentication(); app.UseAuthorization(); @@ -98,9 +99,8 @@ public static void Main(string[] args) app.UseCors(policy => { - policy.SetIsOriginAllowed(origin => true); + policy.SetIsOriginAllowed(origin => true).AllowAnyHeader(); }); - app.UseSession(); StartHangfire(app); app.Run(); diff --git a/rubberduckvba.client/src/app/app.module.ts b/rubberduckvba.client/src/app/app.module.ts index 197ee9e..0427b9a 100644 --- a/rubberduckvba.client/src/app/app.module.ts +++ b/rubberduckvba.client/src/app/app.module.ts @@ -3,7 +3,7 @@ import { CommonModule } from '@angular/common'; import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; import { DataService } from './services/data.service'; -import { ApiClientService } from './services/api-client.service'; +import { AdminApiClientService, ApiClientService } from './services/api-client.service'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { FontAwesomeModule } from '@fortawesome/angular-fontawesome'; import { BrowserModule } from '@angular/platform-browser'; @@ -33,6 +33,8 @@ import { AnnotationComponent } from './routes/annotation/annotation.component'; import { QuickFixComponent } from './routes/quickfixes/quickfix.component'; import { DefaultUrlSerializer, UrlTree } from '@angular/router'; +import { AuthMenuComponent } from './components/auth-menu/auth-menu.component'; +import { AuthComponent } from './routes/auth/auth.component'; /** * https://stackoverflow.com/a/39560520 @@ -52,6 +54,7 @@ export class LowerCaseUrlSerializer extends DefaultUrlSerializer { declarations: [ AppComponent, HomeComponent, + AuthComponent, FeaturesComponent, FeatureComponent, TagDownloadComponent, @@ -70,7 +73,8 @@ export class LowerCaseUrlSerializer extends DefaultUrlSerializer { InspectionComponent, AnnotationComponent, QuickFixComponent, - AboutComponent + AboutComponent, + AuthMenuComponent ], bootstrap: [AppComponent], imports: [ @@ -83,7 +87,8 @@ export class LowerCaseUrlSerializer extends DefaultUrlSerializer { { path: 'inspections/:name', component: InspectionComponent }, { path: 'annotations/:name', component: AnnotationComponent }, { path: 'quickfixes/:name', component: QuickFixComponent }, - { path: 'about', component: AboutComponent}, + { path: 'about', component: AboutComponent }, + { path: 'auth/github', component: AuthComponent }, // legacy routes: { path: 'inspections/details/:name', redirectTo: 'inspections/:name' }, ]), @@ -93,6 +98,7 @@ export class LowerCaseUrlSerializer extends DefaultUrlSerializer { providers: [ DataService, ApiClientService, + AdminApiClientService, provideHttpClient(withInterceptorsFromDi()), { provide: UrlSerializer, diff --git a/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html new file mode 100644 index 0000000..74bd336 --- /dev/null +++ b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html @@ -0,0 +1,98 @@ + +
+ +
+ +
+
+ +
+
+ +
+
+ + +
+
+
+
+
+ + + + + + + + + + + + diff --git a/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts new file mode 100644 index 0000000..ec69b54 --- /dev/null +++ b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts @@ -0,0 +1,79 @@ +import { Component, OnInit, TemplateRef, ViewChild, inject } from "@angular/core"; +import { FaIconLibrary } from "@fortawesome/angular-fontawesome"; +import { BehaviorSubject } from "rxjs"; +import { UserViewModel } from "../../model/feature.model"; +import { AuthService } from "src/app/services/auth.service"; +import { fas } from "@fortawesome/free-solid-svg-icons"; +import { fab } from "@fortawesome/free-brands-svg-icons"; +import { NgbModal } from "@ng-bootstrap/ng-bootstrap"; +import { AdminApiClientService, ApiClientService } from "../../services/api-client.service"; + +@Component({ + selector: 'auth-menu', + templateUrl: './auth-menu.component.html' +}) +export class AuthMenuComponent implements OnInit { + + private readonly _user: BehaviorSubject = new BehaviorSubject(null!); + + public set user(value: UserViewModel) { + this._user.next(value); + } + public get user(): UserViewModel { + return this._user.getValue(); + } + + @ViewChild('confirmbox', { read: TemplateRef }) confirmbox: TemplateRef | undefined; + @ViewChild('confirmtagsbox', { read: TemplateRef }) confirmtagsbox: TemplateRef | undefined; + @ViewChild('confirmxmldocsbox', { read: TemplateRef }) confirmxmldocsbox: TemplateRef | undefined; + public modal = inject(NgbModal); + + constructor(private auth: AuthService, private api: AdminApiClientService, private fa: FaIconLibrary) { + fa.addIconPacks(fas); + fa.addIconPacks(fab); + } + + ngOnInit(): void { + this.getUserInfo(); + } + + getUserInfo(): void { + this.auth.getUser().subscribe(result => { + if (result) { + this._user.next(result); + } + }); + } + + public confirm(): void { + this.modal.open(this.confirmbox); + } + + public confirmUpdateTags(): void { + this.modal.open(this.confirmtagsbox); + } + + public confirmUpdateXmldocs(): void { + this.modal.open(this.confirmxmldocsbox); + } + + public signin(): void { + this.auth.signin(); + this.getUserInfo(); + } + + public signout(): void { + this.auth.signout(); + this.getUserInfo(); + } + + public updateTags(): void { + this.modal.dismissAll(); + this.api.updateTagMetadata(); + } + + public updateXmldocs(): void { + this.modal.dismissAll(); + this.api.updateXmldocMetadata(); + } +} diff --git a/rubberduckvba.client/src/app/components/loading-content/loading-content.component.html b/rubberduckvba.client/src/app/components/loading-content/loading-content.component.html index aa5d462..319c6ec 100644 --- a/rubberduckvba.client/src/app/components/loading-content/loading-content.component.html +++ b/rubberduckvba.client/src/app/components/loading-content/loading-content.component.html @@ -3,6 +3,6 @@ Rubberduck logo
-

loading...

+

{{label}}

diff --git a/rubberduckvba.client/src/app/components/loading-content/loading-content.component.ts b/rubberduckvba.client/src/app/components/loading-content/loading-content.component.ts index 84d0f17..65ec935 100644 --- a/rubberduckvba.client/src/app/components/loading-content/loading-content.component.ts +++ b/rubberduckvba.client/src/app/components/loading-content/loading-content.component.ts @@ -14,4 +14,5 @@ import { AngularDeviceInformationService } from 'angular-device-information'; }) export class LoadingContentComponent { @Input() public show: boolean = true; + @Input() public label: string = 'loading...'; } diff --git a/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.html b/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.html index dde14b2..f9f1a98 100644 --- a/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.html +++ b/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.html @@ -1,67 +1,70 @@
- + + +
+
+ + Windows desktop only :) + +
+
+ + +
+ + + + +
+ + + + + + + + + +
diff --git a/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.ts b/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.ts index 935e365..4a2a35a 100644 --- a/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.ts +++ b/rubberduckvba.client/src/app/components/nav-menu/nav-menu.component.ts @@ -8,6 +8,7 @@ import { filter, map } from 'rxjs/operators'; import { FaIconLibrary } from '@fortawesome/angular-fontawesome'; import { Router, NavigationEnd } from '@angular/router'; import { AngularDeviceInformationService } from 'angular-device-information'; +import { AuthService } from "src/app/services/auth.service"; @Component({ selector: 'app-nav-menu', diff --git a/rubberduckvba.client/src/app/model/feature.model.ts b/rubberduckvba.client/src/app/model/feature.model.ts index e8d4ea7..ae65f33 100644 --- a/rubberduckvba.client/src/app/model/feature.model.ts +++ b/rubberduckvba.client/src/app/model/feature.model.ts @@ -424,3 +424,9 @@ export class AnnotationsFeatureViewModelClass extends SubFeatureViewModelClass i this.annotations = model.annotations.map(e => new AnnotationViewModelClass(e)); } } + +export interface UserViewModel { + name: string; + isAuthenticated: boolean; + isAdmin: boolean; +} diff --git a/rubberduckvba.client/src/app/routes/auth/auth.component.html b/rubberduckvba.client/src/app/routes/auth/auth.component.html new file mode 100644 index 0000000..73c7a3b --- /dev/null +++ b/rubberduckvba.client/src/app/routes/auth/auth.component.html @@ -0,0 +1 @@ + diff --git a/rubberduckvba.client/src/app/routes/auth/auth.component.ts b/rubberduckvba.client/src/app/routes/auth/auth.component.ts new file mode 100644 index 0000000..a0780bd --- /dev/null +++ b/rubberduckvba.client/src/app/routes/auth/auth.component.ts @@ -0,0 +1,16 @@ +import { Component, OnInit } from "@angular/core"; +import { AuthService } from "src/app/services/auth.service"; + +@Component({ + selector: 'app-auth', + templateUrl: './auth.component.html', +}) +export class AuthComponent implements OnInit { + + constructor(private service: AuthService) { + } + + ngOnInit(): void { + this.service.onGithubCallback(); + } +} diff --git a/rubberduckvba.client/src/app/services/api-client.service.ts b/rubberduckvba.client/src/app/services/api-client.service.ts index e315814..1500afd 100644 --- a/rubberduckvba.client/src/app/services/api-client.service.ts +++ b/rubberduckvba.client/src/app/services/api-client.service.ts @@ -1,6 +1,6 @@ import { Injectable } from "@angular/core"; import { LatestTags, Tag } from "../model/tags.model"; -import { AnnotationViewModel, AnnotationViewModelClass, AnnotationsFeatureViewModel, AnnotationsFeatureViewModelClass, FeatureViewModel, FeatureViewModelClass, InspectionViewModel, InspectionViewModelClass, InspectionsFeatureViewModel, InspectionsFeatureViewModelClass, QuickFixViewModel, QuickFixViewModelClass, QuickFixesFeatureViewModel, QuickFixesFeatureViewModelClass, SubFeatureViewModel, SubFeatureViewModelClass, XmlDocOrFeatureViewModel } from "../model/feature.model"; +import { AnnotationViewModel, AnnotationViewModelClass, AnnotationsFeatureViewModel, AnnotationsFeatureViewModelClass, FeatureViewModel, FeatureViewModelClass, InspectionViewModel, InspectionViewModelClass, InspectionsFeatureViewModel, InspectionsFeatureViewModelClass, QuickFixViewModel, QuickFixViewModelClass, QuickFixesFeatureViewModel, QuickFixesFeatureViewModelClass, SubFeatureViewModel, SubFeatureViewModelClass, UserViewModel, XmlDocOrFeatureViewModel } from "../model/feature.model"; import { DownloadInfo } from "../model/downloads.model"; import { DataService } from "./data.service"; import { environment } from "../../environments/environment.prod"; @@ -54,3 +54,26 @@ export class ApiClientService { return this.data.getAsync(url).pipe(map(e => new QuickFixViewModelClass(e))); } } + +@Injectable() +export class AdminApiClientService { + + constructor(private data: DataService) { + } + + public updateTagMetadata(): void { + const url = `${environment.apiBaseUrl}admin/update/tags`; + const jwt = sessionStorage.getItem("jwt"); + if (jwt) { + this.data.postWithAccessTokenAsync(jwt, url); + } + } + + public updateXmldocMetadata(): void { + const url = `${environment.apiBaseUrl}admin/update/xmldoc`; + const jwt = sessionStorage.getItem("jwt"); + if (jwt) { + this.data.postWithAccessTokenAsync(jwt, url); + } + } +} diff --git a/rubberduckvba.client/src/app/services/auth.service.ts b/rubberduckvba.client/src/app/services/auth.service.ts new file mode 100644 index 0000000..2705206 --- /dev/null +++ b/rubberduckvba.client/src/app/services/auth.service.ts @@ -0,0 +1,57 @@ +import { Injectable } from "@angular/core"; +import { Observable } from "rxjs"; +import { environment } from "../../environments/environment"; +import { UserViewModel } from "../model/feature.model"; +import { AuthViewModel, DataService } from "./data.service"; + + +@Injectable({ providedIn: 'root' }) +export class AuthService { + private timeout: number = 10000; + constructor(private data: DataService) { } + + public getUser(): Observable { + const url = `${environment.apiBaseUrl}auth`; + return this.data.getWithAccessTokenAsync(url); + } + + public signin(): void { + const vm = AuthViewModel.withRandomState(); + sessionStorage.setItem('xsrf:state', vm.state); + + const url = `${environment.apiBaseUrl}auth/signin`; + this.data.postAsync(url, vm) + .subscribe(redirectUrl => location.href = redirectUrl); + } + + public signout(): void { + sessionStorage.clear(); + } + + public onGithubCallback(): void { + const urlParams = new URLSearchParams(location.search); + const code: string = urlParams.get('code')!; + const state: string = urlParams.get('state')!; + + if (state === sessionStorage.getItem('xsrf:state')) { + try { + const vm: AuthViewModel = { state, code }; + const url = `${environment.apiBaseUrl}auth/github`; + + this.data.postAsync(url, vm) + .subscribe(result => { + sessionStorage.setItem('github:access_token', result.token!); + location.href = '/'; + }); + } + catch (error) { + console.log(error); + location.href = '/'; + } + } + else { + console.log('xsrf:state mismatched!'); + location.href = '/'; + } + } +} diff --git a/rubberduckvba.client/src/app/services/data.service.ts b/rubberduckvba.client/src/app/services/data.service.ts index 457ed6e..e049ce4 100644 --- a/rubberduckvba.client/src/app/services/data.service.ts +++ b/rubberduckvba.client/src/app/services/data.service.ts @@ -1,7 +1,7 @@ import { HttpClient, HttpHeaders } from "@angular/common/http"; -import { Injectable } from "@angular/core"; +import { Injectable, Query } from "@angular/core"; import { map, timeout, catchError, throwError, Observable } from "rxjs"; -import { environment } from "../../environments/environment"; +import { ApiClientService } from "./api-client.service"; @Injectable() export class DataService { @@ -40,25 +40,62 @@ export class DataService { catchError((err: Response) => throwError(() => err.text)) ); } -} - -@Injectable({ providedIn: 'root' }) -export class AuthService { - constructor(private http: HttpClient) { } - public signin(): Observable { + + public getWithAccessTokenAsync(url: string): Observable { const headers = new HttpHeaders() - .append('accept', 'application/json') - .append('Content-Type', 'application/json; charset=utf-8'); + .append('accept', 'application/json'); + + const token = sessionStorage.getItem('github:access_token'); + if (token) { + headers.append('X-ACCESS-TOKEN', token); + } - return this.http.post(`${environment.apiBaseUrl}auth/signin`, undefined, { headers }); + return this.http.get(url, { headers }) + .pipe( + map(result => result), + timeout(this.timeout), + catchError((err: Response) => { + console.log(err); + return throwError(() => err.text); + }) + ); } - public signout(): Observable { + public postWithAccessTokenAsync(url: string, content?: TContent): Observable { const headers = new HttpHeaders() .append('accept', 'application/json') .append('Content-Type', 'application/json; charset=utf-8'); - return this.http.post(`${environment.apiBaseUrl}auth/signout`, undefined, { headers }); + const token = sessionStorage.getItem('github:access_token'); + if (token) { + headers.append('X-ACCESS-TOKEN', token); + } + + return (content + ? this.http.post(url, content, { headers }) + : this.http.post(url, { headers })) + .pipe( + map(result => result), + timeout(this.timeout), + catchError((err: Response) => throwError(() => err.text)) + ); + } +} + +export class AuthViewModel { + state: string; + code?: string; + token?: string; + + constructor(state: string, code?: string, token?: string) { + this.state = state; + this.code = code; + this.token = token; + } + + public static withRandomState() { + const state = crypto.randomUUID(); + return new AuthViewModel(state); } } From 1453dde2f2e5e164afc9652f720be245eeaee469 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Fri, 7 Feb 2025 04:54:17 -0500 Subject: [PATCH 2/3] tweaks --- .../Api/Auth/AuthController.cs | 4 +- .../GitHubAuthenticationHandler.cs | 13 +++-- rubberduckvba.Server/Program.cs | 8 ++- .../Services/GitHubClientService.cs | 6 +- .../auth-menu/auth-menu.component.html | 20 ++++--- .../auth-menu/auth-menu.component.ts | 7 ++- .../src/app/routes/auth/auth.component.html | 2 +- .../src/app/services/api-client.service.ts | 14 ++--- .../src/app/services/auth.service.ts | 57 +++++++++++-------- .../src/app/services/data.service.ts | 42 ++------------ 10 files changed, 80 insertions(+), 93 deletions(-) diff --git a/rubberduckvba.Server/Api/Auth/AuthController.cs b/rubberduckvba.Server/Api/Auth/AuthController.cs index c0965d7..80a691b 100644 --- a/rubberduckvba.Server/Api/Auth/AuthController.cs +++ b/rubberduckvba.Server/Api/Auth/AuthController.cs @@ -43,8 +43,8 @@ public IActionResult Index() var model = new UserViewModel { Name = name, - IsAuthenticated = isAuthenticated, - IsAdmin = isAuthenticated && role == configuration.Value.OwnerOrg + IsAuthenticated = true, + IsAdmin = role == configuration.Value.OwnerOrg }; return Ok(model); diff --git a/rubberduckvba.Server/GitHubAuthenticationHandler.cs b/rubberduckvba.Server/GitHubAuthenticationHandler.cs index 36dd5ad..3de2c45 100644 --- a/rubberduckvba.Server/GitHubAuthenticationHandler.cs +++ b/rubberduckvba.Server/GitHubAuthenticationHandler.cs @@ -23,15 +23,20 @@ protected async override Task HandleAuthenticateAsync() try { var token = Context.Request.Headers["X-ACCESS-TOKEN"].SingleOrDefault(); - if (token is null) + if (string.IsNullOrWhiteSpace(token)) { return AuthenticateResult.NoResult(); } var principal = await _github.ValidateTokenAsync(token); - return principal is ClaimsPrincipal - ? AuthenticateResult.Success(new AuthenticationTicket(principal, "github")) - : AuthenticateResult.NoResult(); + if (principal is ClaimsPrincipal) + { + Context.User = principal; + Thread.CurrentPrincipal = principal; + return AuthenticateResult.Success(new AuthenticationTicket(principal, "github")); + } + + return AuthenticateResult.NoResult(); } catch (InvalidOperationException e) { diff --git a/rubberduckvba.Server/Program.cs b/rubberduckvba.Server/Program.cs index 425a0e0..e867d05 100644 --- a/rubberduckvba.Server/Program.cs +++ b/rubberduckvba.Server/Program.cs @@ -45,9 +45,7 @@ public static void Main(string[] args) builder.Services.AddAuthentication(options => { options.RequireAuthenticatedSignIn = false; - options.DefaultAuthenticateScheme = "github"; - options.DefaultScheme = "anonymous"; options.AddScheme("github", builder => { @@ -99,7 +97,11 @@ public static void Main(string[] args) app.UseCors(policy => { - policy.SetIsOriginAllowed(origin => true).AllowAnyHeader(); + policy + .AllowAnyMethod() + .AllowAnyHeader() + .AllowCredentials() + .SetIsOriginAllowed(origin => true); }); StartHangfire(app); diff --git a/rubberduckvba.Server/Services/GitHubClientService.cs b/rubberduckvba.Server/Services/GitHubClientService.cs index 80e6f49..6260e96 100644 --- a/rubberduckvba.Server/Services/GitHubClientService.cs +++ b/rubberduckvba.Server/Services/GitHubClientService.cs @@ -44,11 +44,11 @@ public class GitHubClientService(IOptions configuration, ILogger var user = await client.User.Current(); var identity = new ClaimsIdentity(new[] { - new Claim(ClaimTypes.Name, user.Name), - new Claim(ClaimTypes.Email, user.Email), + new Claim(ClaimTypes.Name, user.Login), new Claim(ClaimTypes.Role, config.OwnerOrg), + new Claim(ClaimTypes.Authentication, token), new Claim("access_token", token) - }); + }, "github"); return new ClaimsPrincipal(identity); } diff --git a/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html index 74bd336..7667f64 100644 --- a/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html +++ b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.html @@ -1,23 +1,27 @@
- +
-
-
+
+
+
+
+
+
@@ -44,7 +48,7 @@
Res

@@ -67,7 +71,7 @@
Confirm

@@ -90,7 +94,7 @@
Confirm

diff --git a/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts index ec69b54..eb1c88b 100644 --- a/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts +++ b/rubberduckvba.client/src/app/components/auth-menu/auth-menu.component.ts @@ -1,3 +1,4 @@ +/// import { Component, OnInit, TemplateRef, ViewChild, inject } from "@angular/core"; import { FaIconLibrary } from "@fortawesome/angular-fontawesome"; import { BehaviorSubject } from "rxjs"; @@ -5,7 +6,7 @@ import { UserViewModel } from "../../model/feature.model"; import { AuthService } from "src/app/services/auth.service"; import { fas } from "@fortawesome/free-solid-svg-icons"; import { fab } from "@fortawesome/free-brands-svg-icons"; -import { NgbModal } from "@ng-bootstrap/ng-bootstrap"; +import { NgbModal, NgbToast } from "@ng-bootstrap/ng-bootstrap"; import { AdminApiClientService, ApiClientService } from "../../services/api-client.service"; @Component({ @@ -69,11 +70,11 @@ export class AuthMenuComponent implements OnInit { public updateTags(): void { this.modal.dismissAll(); - this.api.updateTagMetadata(); + this.api.updateTagMetadata().subscribe(jobId => console.log(`UpdateTagMetadata has scheduled job id ${jobId}`)); } public updateXmldocs(): void { this.modal.dismissAll(); - this.api.updateXmldocMetadata(); + this.api.updateXmldocMetadata().subscribe(jobId => console.log(`UpdateXmldocMetadata has scheduled job id ${jobId}`)); } } diff --git a/rubberduckvba.client/src/app/routes/auth/auth.component.html b/rubberduckvba.client/src/app/routes/auth/auth.component.html index 73c7a3b..518cc7a 100644 --- a/rubberduckvba.client/src/app/routes/auth/auth.component.html +++ b/rubberduckvba.client/src/app/routes/auth/auth.component.html @@ -1 +1 @@ - + diff --git a/rubberduckvba.client/src/app/services/api-client.service.ts b/rubberduckvba.client/src/app/services/api-client.service.ts index 1500afd..69584e0 100644 --- a/rubberduckvba.client/src/app/services/api-client.service.ts +++ b/rubberduckvba.client/src/app/services/api-client.service.ts @@ -61,19 +61,13 @@ export class AdminApiClientService { constructor(private data: DataService) { } - public updateTagMetadata(): void { + public updateTagMetadata(): Observable { const url = `${environment.apiBaseUrl}admin/update/tags`; - const jwt = sessionStorage.getItem("jwt"); - if (jwt) { - this.data.postWithAccessTokenAsync(jwt, url); - } + return this.data.postAsync(url); } - public updateXmldocMetadata(): void { + public updateXmldocMetadata(): Observable { const url = `${environment.apiBaseUrl}admin/update/xmldoc`; - const jwt = sessionStorage.getItem("jwt"); - if (jwt) { - this.data.postWithAccessTokenAsync(jwt, url); - } + return this.data.postAsync(url); } } diff --git a/rubberduckvba.client/src/app/services/auth.service.ts b/rubberduckvba.client/src/app/services/auth.service.ts index 2705206..db807a2 100644 --- a/rubberduckvba.client/src/app/services/auth.service.ts +++ b/rubberduckvba.client/src/app/services/auth.service.ts @@ -10,14 +10,25 @@ export class AuthService { private timeout: number = 10000; constructor(private data: DataService) { } + private sleep(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); + } + + private writeStorage(key: string, value: string): void { + sessionStorage.setItem(key, value); + while (sessionStorage.getItem(key) != value) { + this.sleep(1000); + } + } + public getUser(): Observable { const url = `${environment.apiBaseUrl}auth`; - return this.data.getWithAccessTokenAsync(url); + return this.data.getAsync(url); } public signin(): void { const vm = AuthViewModel.withRandomState(); - sessionStorage.setItem('xsrf:state', vm.state); + this.writeStorage('xsrf:state', vm.state); const url = `${environment.apiBaseUrl}auth/signin`; this.data.postAsync(url, vm) @@ -28,30 +39,30 @@ export class AuthService { sessionStorage.clear(); } - public onGithubCallback(): void { - const urlParams = new URLSearchParams(location.search); - const code: string = urlParams.get('code')!; - const state: string = urlParams.get('state')!; + public onGithubCallback(): void { + const urlParams = new URLSearchParams(location.search); + const code: string = urlParams.get('code')!; + const state: string = urlParams.get('state')!; - if (state === sessionStorage.getItem('xsrf:state')) { - try { - const vm: AuthViewModel = { state, code }; - const url = `${environment.apiBaseUrl}auth/github`; - - this.data.postAsync(url, vm) - .subscribe(result => { - sessionStorage.setItem('github:access_token', result.token!); - location.href = '/'; - }); - } - catch (error) { - console.log(error); - location.href = '/'; - } + if (state === sessionStorage.getItem('xsrf:state')) { + try { + const vm: AuthViewModel = { state, code }; + const url = `${environment.apiBaseUrl}auth/github`; + + this.data.postAsync(url, vm) + .subscribe(result => { + this.writeStorage('github:access_token', result.token!); + location.href = '/'; + }); } - else { - console.log('xsrf:state mismatched!'); + catch (error) { + console.log(error); location.href = '/'; } } + else { + console.log('xsrf:state mismatched!'); + location.href = '/'; + } + } } diff --git a/rubberduckvba.client/src/app/services/data.service.ts b/rubberduckvba.client/src/app/services/data.service.ts index e049ce4..9721dd5 100644 --- a/rubberduckvba.client/src/app/services/data.service.ts +++ b/rubberduckvba.client/src/app/services/data.service.ts @@ -12,43 +12,13 @@ export class DataService { } public getAsync(url: string): Observable { - const headers = new HttpHeaders() - .append('accept', 'application/json'); - - return this.http.get(url, { headers }) - .pipe( - map(result => result), - timeout(this.timeout), - catchError((err: Response) => { - console.log(err); - return throwError(() => err.text); - }) - ); - } - - public postAsync(url: string, content?: TContent): Observable { - const headers = new HttpHeaders() - .append('accept', 'application/json') - .append('Content-Type', 'application/json; charset=utf-8'); - - return (content - ? this.http.post(url, content, { headers }) - : this.http.post(url, { headers })) - .pipe( - map(result => result), - timeout(this.timeout), - catchError((err: Response) => throwError(() => err.text)) - ); - } - - - public getWithAccessTokenAsync(url: string): Observable { - const headers = new HttpHeaders() + let headers = new HttpHeaders() .append('accept', 'application/json'); const token = sessionStorage.getItem('github:access_token'); if (token) { - headers.append('X-ACCESS-TOKEN', token); + headers = headers.append('X-ACCESS-TOKEN', token) + .append('Access-Control-Allow-Origin', '*'); } return this.http.get(url, { headers }) @@ -62,14 +32,14 @@ export class DataService { ); } - public postWithAccessTokenAsync(url: string, content?: TContent): Observable { - const headers = new HttpHeaders() + public postAsync(url: string, content?: TContent): Observable { + let headers = new HttpHeaders() .append('accept', 'application/json') .append('Content-Type', 'application/json; charset=utf-8'); const token = sessionStorage.getItem('github:access_token'); if (token) { - headers.append('X-ACCESS-TOKEN', token); + headers = headers.append('X-ACCESS-TOKEN', token); } return (content From 7b386462e105fa745ca4acaee37a69bef4b39400 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Fri, 7 Feb 2025 04:56:18 -0500 Subject: [PATCH 3/3] undo debug tweak --- rubberduckvba.Server/Api/Auth/AuthController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rubberduckvba.Server/Api/Auth/AuthController.cs b/rubberduckvba.Server/Api/Auth/AuthController.cs index 80a691b..3e76251 100644 --- a/rubberduckvba.Server/Api/Auth/AuthController.cs +++ b/rubberduckvba.Server/Api/Auth/AuthController.cs @@ -43,7 +43,7 @@ public IActionResult Index() var model = new UserViewModel { Name = name, - IsAuthenticated = true, + IsAuthenticated = isAuthenticated, IsAdmin = role == configuration.Value.OwnerOrg };