Skip to content

Commit 76a48d3

Browse files
retailcoderretailcoder
committed
added review diffs
1 parent 347685b commit 76a48d3

File tree

12 files changed

+232
-42
lines changed

12 files changed

+232
-42
lines changed

‎rubberduckvba.Server/Api/Auth/AuthController.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,16 @@ public IActionResult OnGitHubCallback(SignInViewModel vm)
115115
{
116116
return GuardInternalAction(() =>
117117
{
118-
Logger.LogInformation("OAuth token was received. State: {state}", vm.State);
118+
Logger.LogInformation("OAuth code was received. State: {state}", vm.State);
119119
var clientId = configuration.Value.ClientId;
120120
var clientSecret = configuration.Value.ClientSecret;
121121
var agent = configuration.Value.UserAgent;
122122

123123
var github = new GitHubClient(new ProductHeaderValue(agent));
124-
125124
var request = new OauthTokenRequest(clientId, clientSecret, vm.Code);
125+
126126
var token = github.Oauth.CreateAccessToken(request).GetAwaiter().GetResult();
127+
127128
if (token is null)
128129
{
129130
Logger.LogWarning("OAuth access token was not created.");
@@ -171,6 +172,13 @@ public IActionResult OnGitHubCallback(SignInViewModel vm)
171172
Thread.CurrentPrincipal = HttpContext.User;
172173

173174
Logger.LogInformation("GitHub user with login {login} has signed in with role authorizations '{role}'.", githubUser.Login, configuration.Value.OwnerOrg);
175+
Response.Cookies.Append(GitHubAuthenticationHandler.AuthCookie, token, new CookieOptions
176+
{
177+
IsEssential = true,
178+
HttpOnly = true,
179+
Secure = true,
180+
Expires = DateTimeOffset.UtcNow.AddHours(1)
181+
});
174182
return token;
175183
}
176184
else

‎rubberduckvba.Server/GitHubAuthenticationHandler.cs

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-

2-
using Microsoft.AspNetCore.Authentication;
1+
using Microsoft.AspNetCore.Authentication;
32
using Microsoft.Extensions.Caching.Memory;
43
using Microsoft.Extensions.Options;
54
using rubberduckvba.Server.Services;
@@ -10,6 +9,10 @@ namespace rubberduckvba.Server;
109

1110
public class GitHubAuthenticationHandler : AuthenticationHandler<AuthenticationSchemeOptions>
1211
{
12+
public static readonly string AuthCookie = "x-access-token";
13+
14+
private static readonly object _lock = new object();
15+
1316
private readonly IGitHubClientService _github;
1417
private readonly IMemoryCache _cache;
1518

@@ -26,45 +29,86 @@ public GitHubAuthenticationHandler(IGitHubClientService github, IMemoryCache cac
2629
_cache = cache;
2730
}
2831

29-
protected async override Task<AuthenticateResult> HandleAuthenticateAsync()
32+
protected override Task<AuthenticateResult> HandleAuthenticateAsync()
3033
{
3134
try
3235
{
33-
var token = Context.Request.Headers["X-ACCESS-TOKEN"].SingleOrDefault();
36+
var token = Context.Request.Cookies[AuthCookie]
37+
?? Context.Request.Headers[AuthCookie];
38+
3439
if (string.IsNullOrWhiteSpace(token))
3540
{
36-
return AuthenticateResult.Fail("Access token was not provided");
41+
return Task.FromResult(AuthenticateResult.Fail("Access token was not provided"));
3742
}
3843

39-
if (_cache.TryGetValue(token, out var cached) && cached is AuthenticationTicket cachedTicket)
44+
if (TryAuthenticateFromCache(token, out var cachedResult))
4045
{
41-
var cachedPrincipal = cachedTicket.Principal;
42-
43-
Context.User = cachedPrincipal;
44-
Thread.CurrentPrincipal = cachedPrincipal;
45-
46-
Logger.LogInformation($"Successfully retrieved authentication ticket from cached token for {cachedPrincipal.Identity!.Name}; token will not be revalidated.");
47-
return AuthenticateResult.Success(cachedTicket);
46+
return Task.FromResult(cachedResult)!;
4847
}
4948

50-
var principal = await _github.ValidateTokenAsync(token);
51-
if (principal is ClaimsPrincipal)
49+
lock (_lock)
5250
{
53-
Context.User = principal;
54-
Thread.CurrentPrincipal = principal;
55-
56-
var ticket = new AuthenticationTicket(principal, "github");
57-
_cache.Set(token, ticket, _options);
51+
if (TryAuthenticateGitHubToken(token, out var result)
52+
&& result is AuthenticateResult
53+
&& result.Ticket is AuthenticationTicket ticket)
54+
{
55+
CacheAuthenticatedTicket(token, ticket);
56+
return Task.FromResult(result!);
57+
}
5858

59-
return AuthenticateResult.Success(ticket);
59+
if (TryAuthenticateFromCache(token, out cachedResult))
60+
{
61+
return Task.FromResult(cachedResult!);
62+
}
6063
}
6164

62-
return AuthenticateResult.Fail("An invalid access token was provided");
65+
return Task.FromResult(AuthenticateResult.Fail("Missing or invalid access token"));
6366
}
6467
catch (InvalidOperationException e)
6568
{
6669
Logger.LogError(e, e.Message);
67-
return AuthenticateResult.NoResult();
70+
return Task.FromResult(AuthenticateResult.NoResult());
71+
}
72+
}
73+
74+
private void CacheAuthenticatedTicket(string token, AuthenticationTicket ticket)
75+
{
76+
if (!string.IsNullOrWhiteSpace(token) && ticket.Principal.Identity?.IsAuthenticated == true)
77+
{
78+
_cache.Set(token, ticket, _options);
79+
}
80+
}
81+
82+
private bool TryAuthenticateFromCache(string token, out AuthenticateResult? result)
83+
{
84+
result = null;
85+
if (_cache.TryGetValue(token, out var cached) && cached is AuthenticationTicket cachedTicket)
86+
{
87+
var cachedPrincipal = cachedTicket.Principal;
88+
89+
Context.User = cachedPrincipal;
90+
Thread.CurrentPrincipal = cachedPrincipal;
91+
92+
Logger.LogInformation($"Successfully retrieved authentication ticket from cached token for {cachedPrincipal.Identity!.Name}; token will not be revalidated.");
93+
result = AuthenticateResult.Success(cachedTicket);
94+
return true;
95+
}
96+
return false;
97+
}
98+
99+
private bool TryAuthenticateGitHubToken(string token, out AuthenticateResult? result)
100+
{
101+
result = null;
102+
var principal = _github.ValidateTokenAsync(token).GetAwaiter().GetResult();
103+
if (principal is ClaimsPrincipal)
104+
{
105+
Context.User = principal;
106+
Thread.CurrentPrincipal = principal;
107+
108+
var ticket = new AuthenticationTicket(principal, "github");
109+
result = AuthenticateResult.Success(ticket);
110+
return true;
68111
}
112+
return false;
69113
}
70114
}

‎rubberduckvba.Server/Services/RubberduckDbService.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,12 @@ private async Task SubmitFeatureEdit(Feature feature, IIdentity identity)
177177

178178
foreach (var name in editableFields)
179179
{
180-
valueBefore = current?.GetType().GetProperty(name)?.GetValue(current)?.ToString();
181-
valueAfter = feature.GetType().GetProperty(name)?.GetValue(feature)?.ToString() ?? string.Empty;
180+
var currentProperty = current.GetType().GetProperty(name);
181+
var property = feature.GetType().GetProperty(name)!;
182+
var asJson = property.PropertyType.IsClass && property.PropertyType != typeof(string);
183+
184+
valueBefore = asJson ? JsonSerializer.Serialize(currentProperty?.GetValue(current)) : currentProperty?.GetValue(current)?.ToString() ?? string.Empty;
185+
valueAfter = asJson ? JsonSerializer.Serialize(property?.GetValue(feature)) : property?.GetValue(feature)?.ToString() ?? string.Empty;
182186

183187
if (valueBefore != valueAfter)
184188
{

‎rubberduckvba.client/package-lock.json

Lines changed: 13 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎rubberduckvba.client/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
"@popperjs/core": "^2.11.6",
3232
"angular-device-information": "^4.0.0",
3333
"bootstrap": "^5.2.3",
34+
"diff": "^8.0.2",
35+
"html-entities": "^2.6.0",
3436
"jest-editor-support": "*",
3537
"run-script-os": "*",
3638
"rxjs": "~7.8.0",

‎rubberduckvba.client/src/app/app.module.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { NgModule } from '@angular/core';
22
import { CommonModule } from '@angular/common';
33
import { FormsModule } from '@angular/forms';
4-
import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
4+
import { NgbAccordionDirective, NgbModule } from '@ng-bootstrap/ng-bootstrap';
55
import { FontAwesomeModule } from '@fortawesome/angular-fontawesome';
66
import { BrowserModule } from '@angular/platform-browser';
77
import { RouterModule, UrlSerializer } from '@angular/router';
@@ -88,6 +88,7 @@ export class LowerCaseUrlSerializer extends DefaultUrlSerializer {
8888
bootstrap: [AppComponent],
8989
imports: [
9090
CommonModule,
91+
NgbModule,
9192
BrowserModule,
9293
FormsModule,
9394
RouterModule.forRoot([
@@ -111,6 +112,7 @@ export class LowerCaseUrlSerializer extends DefaultUrlSerializer {
111112
providers: [
112113
DataService,
113114
ApiClientService,
115+
NgbAccordionDirective,
114116
provideHttpClient(withInterceptorsFromDi()),
115117
{
116118
provide: UrlSerializer,

‎rubberduckvba.client/src/app/routes/audits/audits.component.css

Whitespace-only changes.

‎rubberduckvba.client/src/app/routes/audits/audits.component.html

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,68 @@ <h2>Audits</h2>
55
</p>
66
</div>
77
<div class="row">
8-
<h3>TODO</h3>
8+
<h3>Edits</h3>
9+
<div class="col-12">
10+
<p>
11+
Edits are changes to existing features, such as changing the description, or changing the title.
12+
</p>
13+
<div *ngIf="pendingAudits.edits.length == 0">
14+
<p class="text-muted">There are no pending edits.</p>
15+
</div>
16+
<div ngbAccordion>
17+
<div ngbAccordionItem *ngFor="let edit of pendingAudits.edits" class="mb-2" [collapsed]="false">
18+
<div ngbAccordionHeader>
19+
<h4>{{edit.featureName}}</h4>
20+
<p><span class="fw-bold">{{edit.author}}</span> wants to edit the '{{edit.fieldName.toLowerCase()}}' field.</p>
21+
<p>Submitted <span class="fw-bold">{{edit.dateInserted}}</span> <span *ngIf="edit.dateModified != null">, last modified <span class="fw-bold">{{edit.dateModified}}</span></span></p>
22+
</div>
23+
<div ngbAccordionCollapse>
24+
<div ngbAccordionBody>
25+
<div class="vbe-mock-debugger">
26+
<div class="code-area">
27+
<div [innerHtml]="getDiffHtml(edit.valueBefore ?? '', edit.valueAfter)"></div>
28+
</div>
29+
</div>
30+
</div>
31+
</div>
32+
<button class="btn btn-success" (click)="onApproveEdit(edit)">Approve</button>
33+
<button class="btn btn-outline-danger" (click)="onRejectEdit(edit)">Reject</button>
34+
</div>
35+
</div>
36+
</div>
37+
</div>
38+
<div class="row">
39+
<h3>Operations</h3>
40+
<div class="col-12">
41+
<p>
42+
Operations are additions or deletions of features, such as adding a new feature, or deleting an existing feature.
43+
</p>
44+
<div *ngIf="pendingAudits.other.length == 0">
45+
<p class="text-muted">There are no pending operations.</p>
46+
</div>
47+
<div ngbAccordion>
48+
<div ngbAccordionItem *ngFor="let op of pendingAudits.other" class="mb-2" [collapsed]="false">
49+
<div ngbAccordionHeader>
50+
<h4>{{op.featureName}}</h4>
51+
<div *ngIf="op.featureAction == deleteOp">
52+
<p><span class="fw-bold">{{op.author}}</span> wants to DELETE the '<span class="fw-bold">{{op.featureName}}</span>' feature.</p>
53+
<p>Submitted <span class="fw-bold">{{op.dateInserted}}</span> <span *ngIf="op.dateModified != null">, last modified <span class="fw-bold">{{op.dateModified}}</span></span></p>
54+
</div>
55+
<div *ngIf="op.featureAction == createOp">
56+
<p><span class="fw-bold">{{op.author}}</span> wants to add a new '<span class="fw-bold">{{op.featureName}}</span>' feature.</p>
57+
<p>Submitted <span class="fw-bold">{{op.dateInserted}}</span> <span *ngIf="op.dateModified != null">, last modified <span class="fw-bold">{{op.dateModified}}</span></span></p>
58+
</div>
59+
</div>
60+
<div ngbAccordionCollapse>
61+
<div ngbAccordionBody>
62+
<h3>{{op.title}}</h3>
63+
<p [innerText]="op.shortDescription"></p>
64+
<p [innerHTML]="op.description"></p>
65+
<button class="btn btn-success" (click)="onApproveOperation(op)">Approve</button>
66+
<button class="btn btn-outline-danger" (click)="onRejectOperation(op)">Reject</button>
67+
</div>
68+
</div>
69+
</div>
70+
</div>
71+
</div>
972
</div>

‎rubberduckvba.client/src/app/routes/audits/audits.component.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import { Component, OnInit } from "@angular/core";
22
import { ApiClientService } from "../../services/api-client.service";
3-
import { PendingAuditsViewModel } from "../../model/feature.model";
3+
import { FeatureEditViewModel, FeatureOperation, FeatureOperationViewModel, PendingAuditsViewModel } from "../../model/feature.model";
4+
import { Change, diffWords } from "diff";
5+
import { encode } from "html-entities";
46

57
@Component({
68
selector: 'app-audits',
7-
templateUrl: './audits.component.html'
9+
templateUrl: './audits.component.html',
10+
styleUrls: ['./audits.component.css']
811
})
912
export class AuditsAdminComponent implements OnInit {
1013

@@ -17,5 +20,51 @@ export class AuditsAdminComponent implements OnInit {
1720
ngOnInit(): void {
1821
this.api.getAllPendingAudits().subscribe(e => this.pendingAudits = e);
1922
}
20-
23+
24+
public get deleteOp() { return FeatureOperation.Delete; }
25+
public get createOp() { return FeatureOperation.Create; }
26+
27+
public getDiffHtml(before: string, after: string): string {
28+
const diff = diffWords(before, after, { ignoreCase: false });
29+
return diff.map((part: Change) => {
30+
const htmlEncodedValue = encode(part.value);
31+
if (part.added) {
32+
console.log(`added: ${part.value}`);
33+
return `<span class="text-diff-added">${htmlEncodedValue}</span>`;
34+
} else if (part.removed) {
35+
console.log(`removed: ${part.value}`);
36+
return `<span class="text-diff-removed">${htmlEncodedValue}</span>`;
37+
} else {
38+
return part.value;
39+
}
40+
}).join('');
41+
}
42+
43+
public onApproveEdit(edit:FeatureEditViewModel): void {
44+
this.api.approvePendingAudit(edit.id).subscribe(() => {
45+
console.log(`Feature edit audit record id ${edit.id} was approved.`);
46+
window.location.reload();
47+
});
48+
}
49+
50+
public onRejectEdit(edit:FeatureEditViewModel): void {
51+
this.api.rejectPendingAudit(edit.id).subscribe(() => {
52+
console.log(`Feature edit audit record id ${edit.id} was rejected.`);
53+
window.location.reload();
54+
});
55+
}
56+
57+
public onApproveOperation(op: FeatureOperationViewModel): void {
58+
this.api.approvePendingAudit(op.id).subscribe(() => {
59+
console.log(`Feature operation audit record id ${op.id} was approved.`);
60+
window.location.reload();
61+
});
62+
}
63+
64+
public onRejectOperation(op: FeatureOperationViewModel): void {
65+
this.api.rejectPendingAudit(op.id).subscribe(() => {
66+
console.log(`Feature operation audit record id ${op.id} was rejected.`);
67+
window.location.reload();
68+
});
69+
}
2170
}

0 commit comments

Comments
 (0)