diff --git a/.github/dependabot.yml b/.github/dependabot.yml index e8d8d4de..c6ce9e01 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -16,6 +16,23 @@ updates: - dependencies commit-message: prefix: deps + ignore: + # Microsoft.OpenApi 3.x cannot be used while Microsoft.AspNetCore.OpenApi targets the 2.x + # object model. Its source generator assigns IOpenApiMediaType.Example, which became + # read-only in 3.0, so the build fails in generated code no edit here can reach: + # OpenApiXmlCommentSupport.generated.cs: error CS0200: Property or indexer + # IOpenApiMediaType.Example cannot be assigned to -- it is read only + # + # The direct reference exists only to clear GHSA-v5pm-xwqc-g5wc, which + # Microsoft.AspNetCore.OpenApi 10.0.10 reintroduces by pinning 2.0.0 transitively (see the + # comment in Pgan.PoracleWebNet.Api.csproj). Minor and patch updates inside 2.x still come + # through, so a later advisory is not masked. + # + # Drop this once Microsoft.AspNetCore.OpenApi ships a release built against 3.x -- at which + # point the direct reference should go too. See #702. + - dependency-name: Microsoft.OpenApi + update-types: + - version-update:semver-major groups: # One group for the whole .NET platform. These packages ship as a single versioned set: # Microsoft.EntityFrameworkCore 10.0.x transitively requires Microsoft.Extensions.* at diff --git a/Applications/Pgan.PoracleWebNet.Api/Controllers/AuthController.cs b/Applications/Pgan.PoracleWebNet.Api/Controllers/AuthController.cs index 53b5d665..6091513d 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Controllers/AuthController.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Controllers/AuthController.cs @@ -820,7 +820,16 @@ public async Task Me() // service looked at admin_disable, so an existing token kept full read/write access for the rest of // its 24-hour life, and a fresh login minted another one. The SPA signs out on 401, so answering // that here ends the session on the next poll, the same way a deleted account does. See #597. - if (human.AdminDisable == 1) + // + // Not while impersonating. This 401 ends the CALLER's session, and under impersonation the caller is + // the admin: inspecting a blocked account signed them out of their own, and the SPA's 401 handler + // discards the stashed admin token with everything else, so there was no way back. Blocked is exactly + // the state an admin inspects an account to confirm -- lapsed subscriptions are why alerts stop -- + // so it is returned as data instead. The SPA already renders a banner from adminDisable. See #706. + // + // The deleted-account 401 above deliberately still fires: there is no account left to show. The SPA + // drops an impersonating admin back to their own token on any 401 rather than ending the session. + if (human.AdminDisable == 1 && !this.IsImpersonating) { return this.Unauthorized(new { error = "This account has been blocked by an administrator." }); } @@ -863,7 +872,7 @@ public async Task Me() // where treating unknown as false stripped admin for the rest of the session (#656); and an // impersonation session, which AdminController deliberately mints with IsAdmin = false and which // would otherwise be re-elevated by resolving the impersonated user's own roles (#663). - bool? resolvedAdmin = roles.Resolved && this.User.FindFirst("impersonatedBy") is null + bool? resolvedAdmin = roles.Resolved && !this.IsImpersonating ? roles.IsAdmin : null; userInfo.IsAdmin = resolvedAdmin ?? this.IsAdmin; diff --git a/Applications/Pgan.PoracleWebNet.Api/Controllers/BaseApiController.cs b/Applications/Pgan.PoracleWebNet.Api/Controllers/BaseApiController.cs index c6af4dce..72aa13e0 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Controllers/BaseApiController.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Controllers/BaseApiController.cs @@ -14,6 +14,18 @@ public abstract class BaseApiController : ControllerBase protected string UserId => this.User.FindFirstValue("userId") ?? throw new UnauthorizedAccessException(); protected int ProfileNo => int.Parse(this.User.FindFirstValue("profileNo") ?? "1", CultureInfo.InvariantCulture); protected bool IsAdmin => this.User.FindFirstValue("isAdmin") == "true"; + + /// + /// True when the caller is an admin (or webhook delegate) inspecting somebody else's account, so + /// names the account being looked at rather than the person looking. + /// + /// + /// Only GenerateImpersonationToken sets the claim, and the JWT is signed, so an inspected + /// user cannot mint one for themselves. Every decision about the CALLER -- their admin rights, + /// whether their session survives -- must consult this before reading the effective id, or it + /// answers a question about the wrong person. See #663, #706. + /// + protected bool IsImpersonating => this.User.FindFirst("impersonatedBy") is not null; protected string Username => this.User.FindFirstValue("username") ?? string.Empty; protected string[] ManagedWebhooks => this.User.FindFirstValue("managedWebhooks") ?.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) ?? []; diff --git a/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileController.cs b/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileController.cs index 9d079fd4..de0c727c 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileController.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileController.cs @@ -274,7 +274,7 @@ public async Task SwitchProfile(int profileNo) // where treating unknown as false stripped admin for the rest of the session (#656); and an // impersonation session, which AdminController deliberately mints with IsAdmin = false and which // would otherwise be re-elevated by resolving the impersonated user's own roles (#663). - bool? resolvedAdmin = roles.Resolved && this.User.FindFirst("impersonatedBy") is null + bool? resolvedAdmin = roles.Resolved && !this.IsImpersonating ? roles.IsAdmin : null; var newToken = this._jwtService.GenerateTokenWithReplacedProfile(this.User, profileNo, resolvedAdmin); diff --git a/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileOverviewController.cs b/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileOverviewController.cs index a54ec3d8..a2300c19 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileOverviewController.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Controllers/ProfileOverviewController.cs @@ -119,7 +119,7 @@ await this.WriteGeographyAsync( // where treating unknown as false stripped admin for the rest of the session (#656); and an // impersonation session, which AdminController deliberately mints with IsAdmin = false and which // would otherwise be re-elevated by resolving the impersonated user's own roles (#663). - bool? resolvedAdmin = roles.Resolved && this.User.FindFirst("impersonatedBy") is null + bool? resolvedAdmin = roles.Resolved && !this.IsImpersonating ? roles.IsAdmin : null; var newToken = this._jwtService.GenerateTokenWithReplacedProfile(this.User, this.ProfileNo, resolvedAdmin); @@ -225,7 +225,7 @@ public async Task ImportProfile([FromBody] ProfileOverviewImportR // where treating unknown as false stripped admin for the rest of the session (#656); and an // impersonation session, which AdminController deliberately mints with IsAdmin = false and which // would otherwise be re-elevated by resolving the impersonated user's own roles (#663). - bool? resolvedAdmin = roles.Resolved && this.User.FindFirst("impersonatedBy") is null + bool? resolvedAdmin = roles.Resolved && !this.IsImpersonating ? roles.IsAdmin : null; var newToken = this._jwtService.GenerateTokenWithReplacedProfile(this.User, this.ProfileNo, resolvedAdmin); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/package-lock.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/package-lock.json index caa4e3aa..23969670 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/package-lock.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/package-lock.json @@ -19,7 +19,7 @@ "@angular/router": "^21.2.19", "@ngx-translate/core": "^18.0.0", "@ngx-translate/http-loader": "^18.0.0", - "@types/leaflet": "^1.9.21", + "@types/leaflet": "^1.9.22", "@types/leaflet-draw": "^1.0.13", "chokidar": "^5.0.0", "leaflet": "^1.9.4", @@ -33,8 +33,8 @@ "@angular-eslint/eslint-plugin-template": "^19.0.0", "@angular-eslint/schematics": "^19.0.0", "@angular-eslint/template-parser": "^19.0.0", - "@angular/build": "^21.2.19", - "@angular/cli": "^21.2.19", + "@angular/build": "^21.2.20", + "@angular/cli": "^21.2.20", "@angular/compiler-cli": "^21.2.19", "@jest/globals": "^30.4.1", "@types/jest": "^30.0.0", @@ -346,9 +346,9 @@ } }, "node_modules/@angular-devkit/core": { - "version": "21.2.19", - "resolved": "https://registry.npmjs.org/@angular-devkit/core/-/core-21.2.19.tgz", - "integrity": "sha512-dtpJMQBz5nhkcIogPmXP/aT2Ak8m/wLRPOSTI/g4vSJSuGiI53PgtWq4/wfQga6E6wdM2XWsblAE89d8w5heQQ==", + "version": "21.2.20", + "resolved": "https://registry.npmjs.org/@angular-devkit/core/-/core-21.2.20.tgz", + "integrity": "sha512-QViRAYFj3jcElWND79hM6y4vTSYhMlJSOjy9nby9JsQaPgetkf039jI2u9Lp+PduE6lysewzf85d1UGsm3eI0A==", "dev": true, "license": "MIT", "dependencies": { @@ -657,14 +657,14 @@ } }, "node_modules/@angular/build": { - "version": "21.2.19", - "resolved": "https://registry.npmjs.org/@angular/build/-/build-21.2.19.tgz", - "integrity": "sha512-emy9mqrTXAwhZzcvx8MaHyz+cUR06PVGxnqy91+bpDxPP9S5x67sPoOkY9y/ETFFhRpB5ULlUxyq0eN/pi6QOg==", + "version": "21.2.20", + "resolved": "https://registry.npmjs.org/@angular/build/-/build-21.2.20.tgz", + "integrity": "sha512-Dl9AX8e3mQpAl4RkmvoNnYRoRSpqMLQBqJYf/quupGLxMpQ55mOBhnGfzmoBLyETFhMUd329tvGtOcicPW/hdA==", "dev": true, "license": "MIT", "dependencies": { "@ampproject/remapping": "2.3.0", - "@angular-devkit/architect": "0.2102.19", + "@angular-devkit/architect": "0.2102.20", "@babel/core": "7.29.7", "@babel/helper-annotate-as-pure": "7.27.3", "@babel/helper-split-export-declaration": "7.24.7", @@ -707,7 +707,7 @@ "@angular/platform-browser": "^21.0.0", "@angular/platform-server": "^21.0.0", "@angular/service-worker": "^21.0.0", - "@angular/ssr": "^21.2.19", + "@angular/ssr": "^21.2.20", "karma": "^6.4.0", "less": "^4.2.0", "ng-packagr": "^21.0.0", @@ -757,13 +757,13 @@ } }, "node_modules/@angular/build/node_modules/@angular-devkit/architect": { - "version": "0.2102.19", - "resolved": "https://registry.npmjs.org/@angular-devkit/architect/-/architect-0.2102.19.tgz", - "integrity": "sha512-cj4tzUMiloLTg5rNf17E8MsvIxCWYoBiBsaj7ns6dgXqT9XCeG+J0TA2t1M+N9uuqfeLd22U/rYoCkADmcircQ==", + "version": "0.2102.20", + "resolved": "https://registry.npmjs.org/@angular-devkit/architect/-/architect-0.2102.20.tgz", + "integrity": "sha512-s7wPFCMrt9mWMr+NWs4T8849snnOE4DcmLb9Set7KtbhV6Z8E7ZASs/wqPOS3KPxLjxTqM6CnkwKhc3Th6DniA==", "dev": true, "license": "MIT", "dependencies": { - "@angular-devkit/core": "21.2.19", + "@angular-devkit/core": "21.2.20", "rxjs": "7.8.2" }, "bin": { @@ -1299,19 +1299,19 @@ } }, "node_modules/@angular/cli": { - "version": "21.2.19", - "resolved": "https://registry.npmjs.org/@angular/cli/-/cli-21.2.19.tgz", - "integrity": "sha512-i78NzvoNonAY17QgzSmqrYnXHmEfraLv4wZ/o/m3efxuz61ZJ+5X/PsCeAhbwBvQfRrPRQaJV2tK9vGjHa+U6w==", + "version": "21.2.20", + "resolved": "https://registry.npmjs.org/@angular/cli/-/cli-21.2.20.tgz", + "integrity": "sha512-ATzRaKSDWIUVHQDU14mO3EVrkoEddfm0b4L/uUJraG2tIevRGYStuzT1jIKlPl8mg+O1cDVIjiqdX114kPHczw==", "dev": true, "license": "MIT", "dependencies": { - "@angular-devkit/architect": "0.2102.19", - "@angular-devkit/core": "21.2.19", - "@angular-devkit/schematics": "21.2.19", + "@angular-devkit/architect": "0.2102.20", + "@angular-devkit/core": "21.2.20", + "@angular-devkit/schematics": "21.2.20", "@inquirer/prompts": "7.10.1", "@listr2/prompt-adapter-inquirer": "3.0.5", "@modelcontextprotocol/sdk": "1.26.0", - "@schematics/angular": "21.2.19", + "@schematics/angular": "21.2.20", "@yarnpkg/lockfile": "1.1.0", "algoliasearch": "5.48.1", "ini": "6.0.0", @@ -1334,13 +1334,13 @@ } }, "node_modules/@angular/cli/node_modules/@angular-devkit/architect": { - "version": "0.2102.19", - "resolved": "https://registry.npmjs.org/@angular-devkit/architect/-/architect-0.2102.19.tgz", - "integrity": "sha512-cj4tzUMiloLTg5rNf17E8MsvIxCWYoBiBsaj7ns6dgXqT9XCeG+J0TA2t1M+N9uuqfeLd22U/rYoCkADmcircQ==", + "version": "0.2102.20", + "resolved": "https://registry.npmjs.org/@angular-devkit/architect/-/architect-0.2102.20.tgz", + "integrity": "sha512-s7wPFCMrt9mWMr+NWs4T8849snnOE4DcmLb9Set7KtbhV6Z8E7ZASs/wqPOS3KPxLjxTqM6CnkwKhc3Th6DniA==", "dev": true, "license": "MIT", "dependencies": { - "@angular-devkit/core": "21.2.19", + "@angular-devkit/core": "21.2.20", "rxjs": "7.8.2" }, "bin": { @@ -1353,13 +1353,13 @@ } }, "node_modules/@angular/cli/node_modules/@angular-devkit/schematics": { - "version": "21.2.19", - "resolved": "https://registry.npmjs.org/@angular-devkit/schematics/-/schematics-21.2.19.tgz", - "integrity": "sha512-AG3Fzh9wJCmKBfxUQOUWaEHMj5Gq2O+Msf1z52aDSxbVhs5/iSQcXGPv/DLdAXu7d4xmQhLouNe9Glaq2omDyw==", + "version": "21.2.20", + "resolved": "https://registry.npmjs.org/@angular-devkit/schematics/-/schematics-21.2.20.tgz", + "integrity": "sha512-XH0BtcqSwHlyLRzvbZccSEe5Hcl7Yex8fXfj3gMVBB5g+KBUm2OGtGN3lDOl5BtrwrZBVnNNXkg0ehNrWzxVgw==", "dev": true, "license": "MIT", "dependencies": { - "@angular-devkit/core": "21.2.19", + "@angular-devkit/core": "21.2.20", "jsonc-parser": "3.3.1", "magic-string": "0.30.21", "ora": "9.3.0", @@ -6018,14 +6018,14 @@ "license": "MIT" }, "node_modules/@schematics/angular": { - "version": "21.2.19", - "resolved": "https://registry.npmjs.org/@schematics/angular/-/angular-21.2.19.tgz", - "integrity": "sha512-eL+UU9eizoadhDB4YEctRmmo0A5iwrSmGzeuEa6akrq8nLGVWM8zO91HTJutkPqGQjelF+UOiOShsQSZAU9SIQ==", + "version": "21.2.20", + "resolved": "https://registry.npmjs.org/@schematics/angular/-/angular-21.2.20.tgz", + "integrity": "sha512-D0MFRofD144Gn+LMPgrRQKazv3sNxfB11kbdU19yN8JyXyRqmB3C8/k3lLA/+LyXVeyLwzIKs7Cw/PSzPUaAxA==", "dev": true, "license": "MIT", "dependencies": { - "@angular-devkit/core": "21.2.19", - "@angular-devkit/schematics": "21.2.19", + "@angular-devkit/core": "21.2.20", + "@angular-devkit/schematics": "21.2.20", "jsonc-parser": "3.3.1" }, "engines": { @@ -6035,13 +6035,13 @@ } }, "node_modules/@schematics/angular/node_modules/@angular-devkit/schematics": { - "version": "21.2.19", - "resolved": "https://registry.npmjs.org/@angular-devkit/schematics/-/schematics-21.2.19.tgz", - "integrity": "sha512-AG3Fzh9wJCmKBfxUQOUWaEHMj5Gq2O+Msf1z52aDSxbVhs5/iSQcXGPv/DLdAXu7d4xmQhLouNe9Glaq2omDyw==", + "version": "21.2.20", + "resolved": "https://registry.npmjs.org/@angular-devkit/schematics/-/schematics-21.2.20.tgz", + "integrity": "sha512-XH0BtcqSwHlyLRzvbZccSEe5Hcl7Yex8fXfj3gMVBB5g+KBUm2OGtGN3lDOl5BtrwrZBVnNNXkg0ehNrWzxVgw==", "dev": true, "license": "MIT", "dependencies": { - "@angular-devkit/core": "21.2.19", + "@angular-devkit/core": "21.2.20", "jsonc-parser": "3.3.1", "magic-string": "0.30.21", "ora": "9.3.0", @@ -6519,9 +6519,9 @@ "license": "MIT" }, "node_modules/@types/leaflet": { - "version": "1.9.21", - "resolved": "https://registry.npmjs.org/@types/leaflet/-/leaflet-1.9.21.tgz", - "integrity": "sha512-TbAd9DaPGSnzp6QvtYngntMZgcRk+igFELwR2N99XZn7RXUdKgsXMR+28bUO0rPsWp8MIu/f47luLIQuSLYv/w==", + "version": "1.9.22", + "resolved": "https://registry.npmjs.org/@types/leaflet/-/leaflet-1.9.22.tgz", + "integrity": "sha512-h3lhECYEKDasG7LFHu+GiHqAvsgLuQvlJvVZzJDGONo3sEL+wUOqSFLnwkZlK0qVxnxbuGFW8iBlJNYs5wgndA==", "license": "MIT", "dependencies": { "@types/geojson": "*" diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/package.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/package.json index fe92872b..5a00e90f 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/package.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/package.json @@ -26,7 +26,7 @@ "@angular/router": "^21.2.19", "@ngx-translate/core": "^18.0.0", "@ngx-translate/http-loader": "^18.0.0", - "@types/leaflet": "^1.9.21", + "@types/leaflet": "^1.9.22", "@types/leaflet-draw": "^1.0.13", "chokidar": "^5.0.0", "leaflet": "^1.9.4", @@ -40,8 +40,8 @@ "@angular-eslint/eslint-plugin-template": "^19.0.0", "@angular-eslint/schematics": "^19.0.0", "@angular-eslint/template-parser": "^19.0.0", - "@angular/build": "^21.2.19", - "@angular/cli": "^21.2.19", + "@angular/build": "^21.2.20", + "@angular/cli": "^21.2.20", "@angular/compiler-cli": "^21.2.19", "@jest/globals": "^30.4.1", "@types/jest": "^30.0.0", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/app.html b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/app.html index 1c85d749..70e3398f 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/app.html +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/app.html @@ -10,7 +10,14 @@ } -@if (auth.user()?.adminDisable) { +@if (auth.user()?.adminDisable && auth.isImpersonating()) { + +
+ block + +
+} @else if (auth.user()?.adminDisable) {
block diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.spec.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.spec.ts index 302f3d8a..7ae5f118 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.spec.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.spec.ts @@ -50,10 +50,9 @@ describe('errorInterceptor', () => { }); it('clears the whole session on a 401, not just the access token', () => { - // The admin token is the higher-privilege credential an impersonating admin leaves behind, and - // stopImpersonating() would install it as the active token. See #616. + // The refresh token and its expiry used to survive the app deciding the session was invalid, so the + // next load tried to refresh a session the server had already rejected. See #616. localStorage.setItem('poracle_token', 'expired-token'); - localStorage.setItem('poracle_admin_token', 'admin-token'); localStorage.setItem('poracle_refresh_token', 'refresh-token'); localStorage.setItem('poracle_token_expires_at', '1'); @@ -61,11 +60,46 @@ describe('errorInterceptor', () => { httpMock.expectOne('/api/dashboard').flush(null, { status: 401, statusText: 'Unauthorized' }); expect(localStorage.getItem('poracle_token')).toBeNull(); - expect(localStorage.getItem('poracle_admin_token')).toBeNull(); expect(localStorage.getItem('poracle_refresh_token')).toBeNull(); expect(localStorage.getItem('poracle_token_expires_at')).toBeNull(); }); + it('ends the inspection, not the session, when a 401 arrives while impersonating', () => { + // Inspecting a blocked or deleted account 401s /api/auth/me, and clearing the session took the + // stashed admin token with it -- signing the admin out of their own session with nothing to go back + // to. The 401 belongs to the account being inspected, not the admin holding the session. See #706. + localStorage.setItem('poracle_token', 'impersonation-token'); + localStorage.setItem('poracle_admin_token', 'admin-token'); + + http.get('/api/dashboard').subscribe({ error: () => {} }); + httpMock.expectOne('/api/dashboard').flush(null, { status: 401, statusText: 'Unauthorized' }); + + expect(localStorage.getItem('poracle_token')).toBe('admin-token'); + expect(localStorage.getItem('poracle_admin_token')).toBeNull(); + expect(router.navigate).toHaveBeenCalledWith(['/admin']); + expect(toast.error).toHaveBeenCalledWith('HTTP_ERROR.INSPECTION_ENDED'); + expect(toast.error).not.toHaveBeenCalledWith('HTTP_ERROR.UNAUTHORIZED'); + }); + + it('falls back only once, so a dead admin token still ends the session', () => { + // The fallback consumes the stash, so the #616 guarantee still lands: the second 401 finds nothing + // to restore and clears everything. Without that it would loop, or strand a session that cannot work. + localStorage.setItem('poracle_token', 'impersonation-token'); + localStorage.setItem('poracle_admin_token', 'expired-admin-token'); + localStorage.setItem('poracle_refresh_token', 'refresh-token'); + + http.get('/api/dashboard').subscribe({ error: () => {} }); + httpMock.expectOne('/api/dashboard').flush(null, { status: 401, statusText: 'Unauthorized' }); + + http.get('/api/dashboard').subscribe({ error: () => {} }); + httpMock.expectOne('/api/dashboard').flush(null, { status: 401, statusText: 'Unauthorized' }); + + expect(localStorage.getItem('poracle_token')).toBeNull(); + expect(localStorage.getItem('poracle_admin_token')).toBeNull(); + expect(localStorage.getItem('poracle_refresh_token')).toBeNull(); + expect(router.navigate).toHaveBeenLastCalledWith(['/login'], { queryParams: {} }); + }); + it('should show permission toast for 403 without disableKey', () => { http.get('/api/admin/users').subscribe({ error: () => {} }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.ts index 31a19a40..c6c724ef 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/interceptors/error.interceptor.ts @@ -40,6 +40,17 @@ export const errorInterceptor: HttpInterceptorFn = (req, next) => { // On 401, clear token and redirect — but NOT during OAuth callback flow or login page if (error.status === 401 && !isAuthCallbackRoute()) { + // A 401 while inspecting another account is that account's problem, not the admin's, so end the + // inspection rather than the session. Without this, inspecting a blocked or deleted user hit the + // clearAll() below and signed the admin out with nothing to return to. See #706. + // Toasted even for the silenced endpoints: unlike a background poll, this one explains a + // navigation the admin can see happen. + if (tokenStore.tryRestoreAdminSession()) { + toast.error(translate.instant('HTTP_ERROR.INSPECTION_ENDED')); + router.navigate(['/admin']); + return throwError(() => error); + } + // The whole session, not just the access token. Three keys used to survive the app deciding the // session was invalid: poracle_admin_token -- the higher-privilege credential an impersonating // admin leaves behind, which stopImpersonating() would then install as the active token -- plus diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.spec.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.spec.ts index 4744ed99..132ebf79 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.spec.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.spec.ts @@ -5,6 +5,7 @@ import { Router } from '@angular/router'; import { AuthService } from './auth.service'; import { ConfigService } from './config.service'; +import { TokenStoreService } from './token-store.service'; import { UserInfo } from '../models'; describe('AuthService', () => { @@ -167,7 +168,10 @@ describe('AuthService', () => { expect(service.isLoggedIn()).toBe(true); }); - it('should clear token and user on 401 error', async () => { + it('should forget the user on 401 error, leaving the token to the interceptor', async () => { + // Removing poracle_token here as well as in the interceptor deleted the admin token the + // impersonation fallback had just restored, one line after it was written. The interceptor owns + // 401 token handling -- clearAll(), or the fallback -- and this only resets the user. See #706. localStorage.setItem('poracle_token', 'bad-token'); const promise = service.loadCurrentUser(); @@ -176,7 +180,6 @@ describe('AuthService', () => { const result = await promise; expect(result).toBeNull(); - expect(localStorage.getItem('poracle_token')).toBeNull(); expect(service.user()).toBeNull(); }); @@ -363,4 +366,24 @@ describe('AuthService', () => { expect(localStorage.getItem('poracle_admin_token')).toBeNull(); }); }); + + describe('an inspection ended by a 401', () => { + it('drops the impersonation state and reloads the admin behind the restored token', () => { + // The interceptor puts the admin's own token back rather than ending the session; without picking + // the user back up, the banner kept naming the inspected account and the nav kept its rights. + // See #706. + const tokenStore = TestBed.inject(TokenStoreService); + localStorage.setItem('poracle_token', 'impersonation-jwt'); + localStorage.setItem('poracle_admin_token', 'admin-jwt'); + service.impersonate('impersonation-jwt'); + httpMock.expectOne(`${API}/api/auth/me`).flush(mockUser); + expect(service.isImpersonating()).toBe(true); + + tokenStore.tryRestoreAdminSession(); + + expect(service.isImpersonating()).toBe(false); + httpMock.expectOne(`${API}/api/auth/me`).flush({ ...mockUser, id: 'admin-1', username: 'admin' }); + expect(service.user()?.username).toBe('admin'); + }); + }); }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.ts index a0ca7c5f..0234d79d 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/auth.service.ts @@ -40,6 +40,13 @@ export class AuthService { // signed-in shell and an impersonation banner around the login page. See #627, #628. this.tokenStore.sessionCleared$.subscribe(() => this.clearSession()); + // A 401 under impersonation drops back to the admin's own token instead of ending the session; + // pick the admin's user back up so the banner and nav match who the token now names. See #706. + this.tokenStore.impersonationEnded$.subscribe(() => { + this._isImpersonating.set(false); + void this.loadCurrentUser(); + }); + const token = localStorage.getItem(TOKEN_KEY); if (token) { this.loadCurrentUser(); @@ -113,7 +120,10 @@ export class AuthService { this.http.get(`${this.config.apiHost}/api/auth/me`).subscribe({ error: err => { if (err.status === 401) { - localStorage.removeItem(TOKEN_KEY); + // Only the user object. The interceptor owns what happens to the tokens on a 401 -- either + // clearAll(), which already empties this via sessionCleared$, or the impersonation fallback + // that installs the admin's own token. Removing poracle_token here as well deleted the token + // that fallback had just restored, one line after it was written. See #706, #616. this.currentUser.set(null); } this.userLoaded$.next(null); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.spec.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.spec.ts index 0eba32dd..9b22f6f0 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.spec.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.spec.ts @@ -137,4 +137,32 @@ describe('TokenStoreService', () => { expect(announced).toBe(true); }); }); + + describe('tryRestoreAdminSession', () => { + it('installs the stashed admin token and announces the end of the inspection', () => { + localStorage.setItem('poracle_token', 'impersonation-jwt'); + localStorage.setItem('poracle_admin_token', 'admin-jwt'); + let announced = false; + service.impersonationEnded$.subscribe(() => (announced = true)); + + expect(service.tryRestoreAdminSession()).toBe(true); + + expect(localStorage.getItem('poracle_token')).toBe('admin-jwt'); + expect(localStorage.getItem('poracle_admin_token')).toBeNull(); + expect(announced).toBe(true); + }); + + it('leaves an ordinary session alone when there is nothing stashed', () => { + // The common case: an expired token on a session that was never impersonating. Touching it here + // would strand a session the 401 path is about to clear anyway. See #706. + localStorage.setItem('poracle_token', 'expired-jwt'); + let announced = false; + service.impersonationEnded$.subscribe(() => (announced = true)); + + expect(service.tryRestoreAdminSession()).toBe(false); + + expect(localStorage.getItem('poracle_token')).toBe('expired-jwt'); + expect(announced).toBe(false); + }); + }); }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.ts index e0f83b06..f6cf1703 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/core/services/token-store.service.ts @@ -36,6 +36,9 @@ export class TokenStoreService { /** Emits when a refresh definitively fails — AuthService subscribes and logs the user out. */ readonly forceLogout$ = new Subject(); + /** Emits when a 401 dropped an impersonation session back to the admin's own token. */ + readonly impersonationEnded$ = new Subject(); + /** Emits when the session was discarded from under the app — AuthService resets its own state. */ readonly sessionCleared$ = new Subject(); @@ -146,6 +149,27 @@ export class TokenStoreService { } } + /** + * Puts the stashed admin token back as the active one, if there is one. Returns whether it did. + */ + /* A 401 belongs to whoever the token names, and while inspecting an account that is the inspected + * user, not the admin holding the session. clearAll() treats every 401 as the end of the session and + * discards poracle_admin_token with the rest, so one blocked or deleted account signed the admin out + * of their own session with nothing to return to -- inspecting exactly the accounts an admin most + * needs to inspect. Falling back is self-limiting: if the restored admin token is itself dead, the + * next 401 finds no stash and clears normally. See #706, #616. */ + tryRestoreAdminSession(): boolean { + const adminToken = localStorage.getItem(ADMIN_TOKEN_KEY); + if (!adminToken) { + return false; + } + + localStorage.setItem(TOKEN_KEY, adminToken); + localStorage.removeItem(ADMIN_TOKEN_KEY); + this.impersonationEnded$.next(); + return true; + } + private decodeExpiry(token: string): number | null { try { const payload = JSON.parse(atob(token.split('.')[1])); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/da.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/da.json index af87fbde..5336c4b9 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/da.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/da.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Viser som", "BACK_TO_ADMIN": "Tilbage til Admin", "DISABLED_ACCOUNT": "Din konto er blevet deaktiveret. Det kan skyldes hastighedsbegrænsning eller en administrativ handling.", + "DISABLED_ACCOUNT_INSPECTED": "Denne konto er blevet deaktiveret af en administrator og modtager ikke notifikationer.", "DISABLED_SUPPORT": "For at få hjælp, spørg i", "PAUSED_ALERTS": "Dine alarmer er sat på pause. Du vil ikke modtage notifikationer.", "RESUME": "Genoptag" @@ -79,6 +80,7 @@ "NETWORK": "Kan ikke nå serveren. Tjek din forbindelse.", "BAD_REQUEST": "Ugyldig forespørgsel. Tjek dine input.", "UNAUTHORIZED": "Din session er udløbet. Log ind igen.", + "INSPECTION_ENDED": "Inspektionen er afsluttet – du er tilbage i din egen session.", "FORBIDDEN": "Du har ikke tilladelse til at udføre denne handling.", "NOT_FOUND": "Den anmodede ressource blev ikke fundet.", "CONFLICT": "Der opstod en konflikt. Elementet kan være blevet ændret.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/de.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/de.json index 335d21be..1e4b32d3 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/de.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/de.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Angezeigt als", "BACK_TO_ADMIN": "Zurück zum Admin", "DISABLED_ACCOUNT": "Dein Konto wurde deaktiviert. Dies kann an einer Ratenbegrenzung oder einer administrativen Maßnahme liegen.", + "DISABLED_ACCOUNT_INSPECTED": "Dieses Konto wurde von einem Administrator deaktiviert und erhält keine Benachrichtigungen.", "DISABLED_SUPPORT": "Für Hilfe frag in", "PAUSED_ALERTS": "Deine Benachrichtigungen sind pausiert. Du wirst keine Benachrichtigungen erhalten.", "RESUME": "Fortsetzen" @@ -79,6 +80,7 @@ "NETWORK": "Server nicht erreichbar. Bitte überprüfe deine Verbindung.", "BAD_REQUEST": "Ungültige Anfrage. Bitte überprüfe deine Eingabe.", "UNAUTHORIZED": "Deine Sitzung ist abgelaufen. Bitte melde dich erneut an.", + "INSPECTION_ENDED": "Die Ansicht wurde beendet – du bist zurück in deiner eigenen Sitzung.", "FORBIDDEN": "Du hast keine Berechtigung für diese Aktion.", "NOT_FOUND": "Die angeforderte Ressource wurde nicht gefunden.", "CONFLICT": "Ein Konflikt ist aufgetreten. Das Element wurde möglicherweise geändert.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/en.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/en.json index 3e8db6ad..5a978262 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/en.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/en.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Viewing as", "BACK_TO_ADMIN": "Back to Admin", "DISABLED_ACCOUNT": "Your account has been disabled. This may be due to rate limiting or an administrative action.", + "DISABLED_ACCOUNT_INSPECTED": "This account has been disabled by an administrator and is not receiving notifications.", "DISABLED_SUPPORT": "To get help, ask in", "PAUSED_ALERTS": "Your alerts are paused. You will not receive notifications.", "RESUME": "Resume" @@ -79,6 +80,7 @@ "NETWORK": "Unable to reach the server. Please check your connection.", "BAD_REQUEST": "Invalid request. Please check your input.", "UNAUTHORIZED": "Your session has expired. Please sign in again.", + "INSPECTION_ENDED": "Inspection ended — you are back in your own session.", "FORBIDDEN": "You do not have permission to perform this action.", "NOT_FOUND": "The requested resource was not found.", "CONFLICT": "A conflict occurred. The item may have been modified.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/es.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/es.json index ffa38f77..95b81869 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/es.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/es.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Viendo como", "BACK_TO_ADMIN": "Volver al Admin", "DISABLED_ACCOUNT": "Tu cuenta ha sido desactivada. Esto puede deberse a un límite de solicitudes o a una acción administrativa.", + "DISABLED_ACCOUNT_INSPECTED": "Esta cuenta ha sido desactivada por un administrador y no recibe notificaciones.", "DISABLED_SUPPORT": "Para obtener ayuda, pregunta en", "PAUSED_ALERTS": "Tus alertas están en pausa. No recibirás notificaciones.", "RESUME": "Reanudar" @@ -79,6 +80,7 @@ "NETWORK": "No se puede conectar al servidor. Verifica tu conexión.", "BAD_REQUEST": "Solicitud inválida. Verifica tus datos.", "UNAUTHORIZED": "Tu sesión ha expirado. Inicia sesión de nuevo.", + "INSPECTION_ENDED": "Se ha finalizado la inspección: has vuelto a tu propia sesión.", "FORBIDDEN": "No tienes permiso para realizar esta acción.", "NOT_FOUND": "El recurso solicitado no fue encontrado.", "CONFLICT": "Ocurrió un conflicto. El elemento puede haber sido modificado.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/fr.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/fr.json index df4ae6c7..9ee16606 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/fr.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/fr.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Vu en tant que", "BACK_TO_ADMIN": "Retour à l'admin", "DISABLED_ACCOUNT": "Ton compte a été désactivé. Cela peut être dû à une limitation de débit ou à une action administrative.", + "DISABLED_ACCOUNT_INSPECTED": "Ce compte a été désactivé par un administrateur et ne reçoit aucune notification.", "DISABLED_SUPPORT": "Pour obtenir de l'aide, demande dans", "PAUSED_ALERTS": "Tes alertes sont en pause. Tu ne recevras pas de notifications.", "RESUME": "Reprendre" @@ -79,6 +80,7 @@ "NETWORK": "Impossible de joindre le serveur. Vérifie ta connexion.", "BAD_REQUEST": "Requête invalide. Vérifie tes données.", "UNAUTHORIZED": "Ta session a expiré. Reconnecte-toi.", + "INSPECTION_ENDED": "Inspection terminée : vous êtes de retour dans votre propre session.", "FORBIDDEN": "Tu n'as pas la permission d'effectuer cette action.", "NOT_FOUND": "La ressource demandée est introuvable.", "CONFLICT": "Un conflit est survenu. L'élément a peut-être été modifié.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/it.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/it.json index b1b5d209..095bcbdb 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/it.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/it.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Visualizzazione come", "BACK_TO_ADMIN": "Torna all'Amministrazione", "DISABLED_ACCOUNT": "Il tuo account è stato disabilitato. Questo potrebbe essere dovuto a un limite di richieste o a un'azione amministrativa.", + "DISABLED_ACCOUNT_INSPECTED": "Questo account è stato disattivato da un amministratore e non riceve notifiche.", "DISABLED_SUPPORT": "Per assistenza, chiedi in", "PAUSED_ALERTS": "I tuoi avvisi sono in pausa. Non riceverai notifiche.", "RESUME": "Riprendi" @@ -79,6 +80,7 @@ "NETWORK": "Impossibile raggiungere il server. Controlla la tua connessione.", "BAD_REQUEST": "Richiesta non valida. Controlla i dati inseriti.", "UNAUTHORIZED": "La tua sessione è scaduta. Accedi di nuovo.", + "INSPECTION_ENDED": "Ispezione terminata: sei tornato alla tua sessione.", "FORBIDDEN": "Non hai i permessi per eseguire questa azione.", "NOT_FOUND": "La risorsa richiesta non è stata trovata.", "CONFLICT": "Si è verificato un conflitto. L'elemento potrebbe essere stato modificato.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/nl.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/nl.json index 18b7ec4d..aaf0b291 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/nl.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/nl.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Bekijken als", "BACK_TO_ADMIN": "Terug naar Beheer", "DISABLED_ACCOUNT": "Je account is uitgeschakeld. Dit kan komen door snelheidsbeperking of een beheerdersactie.", + "DISABLED_ACCOUNT_INSPECTED": "Dit account is door een beheerder uitgeschakeld en ontvangt geen meldingen.", "DISABLED_SUPPORT": "Voor hulp, vraag het in", "PAUSED_ALERTS": "Je meldingen zijn gepauzeerd. Je ontvangt geen notificaties.", "RESUME": "Hervatten" @@ -79,6 +80,7 @@ "NETWORK": "Kan de server niet bereiken. Controleer je verbinding.", "BAD_REQUEST": "Ongeldig verzoek. Controleer je invoer.", "UNAUTHORIZED": "Je sessie is verlopen. Log opnieuw in.", + "INSPECTION_ENDED": "Inspectie beëindigd — je bent terug in je eigen sessie.", "FORBIDDEN": "Je hebt geen toestemming voor deze actie.", "NOT_FOUND": "De gevraagde bron is niet gevonden.", "CONFLICT": "Er is een conflict opgetreden. Het item is mogelijk gewijzigd.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pl.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pl.json index 6ec690fc..7df8267a 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pl.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pl.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Przeglądasz jako", "BACK_TO_ADMIN": "Powrót do Admina", "DISABLED_ACCOUNT": "Twoje konto zostało wyłączone. Może to być spowodowane limitem zapytań lub działaniem administratora.", + "DISABLED_ACCOUNT_INSPECTED": "To konto zostało wyłączone przez administratora i nie otrzymuje powiadomień.", "DISABLED_SUPPORT": "Aby uzyskać pomoc, zapytaj na", "PAUSED_ALERTS": "Twoje alerty są wstrzymane. Nie będziesz otrzymywać powiadomień.", "RESUME": "Wznów" @@ -79,6 +80,7 @@ "NETWORK": "Nie można połączyć się z serwerem. Sprawdź swoje połączenie.", "BAD_REQUEST": "Nieprawidłowe żądanie. Sprawdź wprowadzone dane.", "UNAUTHORIZED": "Twoja sesja wygasła. Zaloguj się ponownie.", + "INSPECTION_ENDED": "Zakończono podgląd — wróciłeś do własnej sesji.", "FORBIDDEN": "Nie masz uprawnień do wykonania tej akcji.", "NOT_FOUND": "Żądany zasób nie został znaleziony.", "CONFLICT": "Wystąpił konflikt. Element mógł zostać zmodyfikowany.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt-BR.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt-BR.json index 863338ff..ce2fb60e 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt-BR.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt-BR.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Visualizando como", "BACK_TO_ADMIN": "Voltar ao Admin", "DISABLED_ACCOUNT": "Sua conta foi desativada. Isso pode ser devido a limite de requisições ou uma ação administrativa.", + "DISABLED_ACCOUNT_INSPECTED": "Esta conta foi desativada por um administrador e não recebe notificações.", "DISABLED_SUPPORT": "Para obter ajuda, pergunte em", "PAUSED_ALERTS": "Seus alertas estão pausados. Você não receberá notificações.", "RESUME": "Retomar" @@ -79,6 +80,7 @@ "NETWORK": "Não foi possível conectar ao servidor. Verifique sua conexão.", "BAD_REQUEST": "Requisição inválida. Verifique seus dados.", "UNAUTHORIZED": "Sua sessão expirou. Faça login novamente.", + "INSPECTION_ENDED": "Inspeção encerrada — você voltou à sua própria sessão.", "FORBIDDEN": "Você não tem permissão para realizar esta ação.", "NOT_FOUND": "O recurso solicitado não foi encontrado.", "CONFLICT": "Ocorreu um conflito. O item pode ter sido modificado.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt.json index 9de7edd2..330a2c62 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/pt.json @@ -41,6 +41,7 @@ "VIEWING_AS": "A ver como", "BACK_TO_ADMIN": "Voltar à Administração", "DISABLED_ACCOUNT": "A tua conta foi desativada. Isto pode ser devido a limitação de pedidos ou a uma ação administrativa.", + "DISABLED_ACCOUNT_INSPECTED": "Esta conta foi desativada por um administrador e não recebe notificações.", "DISABLED_SUPPORT": "Para obter ajuda, pergunta em", "PAUSED_ALERTS": "Os teus alertas estão em pausa. Não vais receber notificações.", "RESUME": "Retomar" @@ -79,6 +80,7 @@ "NETWORK": "Não foi possível contactar o servidor. Verifica a tua ligação.", "BAD_REQUEST": "Pedido inválido. Verifica os dados introduzidos.", "UNAUTHORIZED": "A tua sessão expirou. Inicia sessão novamente.", + "INSPECTION_ENDED": "Inspeção terminada — voltou à sua própria sessão.", "FORBIDDEN": "Não tens permissão para realizar esta ação.", "NOT_FOUND": "O recurso solicitado não foi encontrado.", "CONFLICT": "Ocorreu um conflito. O item pode ter sido modificado.", diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/sv.json b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/sv.json index 470951b6..4486675c 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/sv.json +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/assets/i18n/sv.json @@ -41,6 +41,7 @@ "VIEWING_AS": "Visar som", "BACK_TO_ADMIN": "Tillbaka till Admin", "DISABLED_ACCOUNT": "Ditt konto har inaktiverats. Det kan bero på hastighetsbegränsning eller en administratörsåtgärd.", + "DISABLED_ACCOUNT_INSPECTED": "Det här kontot har inaktiverats av en administratör och tar inte emot aviseringar.", "DISABLED_SUPPORT": "För att få hjälp, fråga i", "PAUSED_ALERTS": "Dina larm är pausade. Du kommer inte att få notiser.", "RESUME": "Återuppta" @@ -79,6 +80,7 @@ "NETWORK": "Kan inte nå servern. Kontrollera din anslutning.", "BAD_REQUEST": "Ogiltig begäran. Kontrollera dina uppgifter.", "UNAUTHORIZED": "Din session har gått ut. Logga in igen.", + "INSPECTION_ENDED": "Inspektionen avslutades – du är tillbaka i din egen session.", "FORBIDDEN": "Du har inte behörighet att utföra denna åtgärd.", "NOT_FOUND": "Den begärda resursen hittades inte.", "CONFLICT": "En konflikt uppstod. Objektet kan ha ändrats.", diff --git a/CHANGELOG.md b/CHANGELOG.md index 40e717f2..32a77978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Inspecting a blocked user no longer signs the admin out.** `/api/auth/me` answers 401 for an account an administrator has blocked, which is how a blocked user's session ends — but under impersonation that 401 lands on the admin doing the inspecting, and the SPA discards the stashed admin token along with the rest of the session, so there was no way back. Lapsed subscribers are blocked accounts, and "why did this person's alerts stop?" is the main reason to inspect one at all, so inspection hit it constantly. The blocked state is now reported as data — the banner says so — and inspection works. Any other 401 while inspecting ends the inspection and returns the admin to their own session rather than logging them out ([#706](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/706)). +- **Expired OIDC refresh sessions are actually deleted now.** The background cleanup had never once completed on MariaDB: EF Core's `ExecuteDeleteAsync` emits ``DELETE FROM `oidc_sessions` AS `o` ``, and MariaDB rejects an aliased single-table delete outright, so every pass since the feature shipped threw a 1064 and logged a warning while the table only grew. The delete is now raw SQL with no alias. Nothing needs doing on upgrade — the first pass after startup clears the backlog. The eight sibling deletes in `HumanRepository` would have failed the same way and are gone; they had been dead code since alarm deletion moved to the PoracleNG proxy ([#707](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/707)). +- Dependabot no longer proposes `Microsoft.OpenApi` 3.x every week. The 3.0 object model made `IOpenApiMediaType.Example` read-only, and `Microsoft.AspNetCore.OpenApi` 10.0.10 still generates code that assigns it, so the bump cannot build and no edit in this repository can reach the failure. Minor and patch updates inside 2.x still come through, so a later advisory is not masked ([#702](https://github.com/PGAN-Dev/PoracleWeb.NET/pull/702)). + ## [2.15.0] - 2026-08-10 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 4b1cd29d..4240014b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -424,6 +424,13 @@ On first startup after upgrade, the `SettingsMigrationStartupService` automatica ### MariaDB GET_LOCK Compatibility `MySql.EntityFrameworkCore`'s `MigrateAsync()` uses `GET_LOCK('__EFMigrationsLock', -1)` which returns NULL on MariaDB (infinite timeout not supported), causing `System.InvalidCastException`. The `MariaDbHistoryRepository` class overrides the lock acquisition to use `GET_LOCK(3600)` instead. This is registered via `ReplaceService()` on `PoracleWebContext`. +### `ExecuteDeleteAsync` Is Unusable On MariaDB +`MySql.EntityFrameworkCore` emits the aliased single-table form, ``DELETE FROM `t` AS `x` WHERE …``, and MariaDB answers 1064 — it requires the multi-table ``DELETE x FROM t AS x`` once an alias is present. `ExecuteUpdateAsync` is fine; MariaDB accepts `UPDATE t AS x SET x.c = …`. Verified against MariaDB 10.8.2. + +Nothing catches this before production. It compiles, and the repository tests pass because they run on **SQLite**, whose provider emits the same alias and accepts it. The OIDC session cleanup shipped this way and had never once run (#707); `QuickPickAppliedStateRepository` hit it earlier and quietly grew a load-and-`RemoveRange` workaround. + +Use raw SQL with **unquoted** identifiers (so the statement also parses on SQLite for the tests), or load and `RemoveRange` when the row count is small. `NoAliasedDeleteTests` fails the build if `.ExecuteDeleteAsync(` reappears anywhere under `Core/`, `Data/` or the API project. + ### Gym ID NULL vs Empty String The `gym_id` column in Poracle alarm tables (gym, raid, egg) is a `NOT NULL` string that defaults to `""` (empty string) meaning "any gym". PoracleNG handles the null-to-empty normalization on its side. The `GymPickerComponent` emits `null` when cleared and the gym's `id` string when selected. diff --git a/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IHumanRepository.cs b/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IHumanRepository.cs index 54bb8d7f..94553568 100644 --- a/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IHumanRepository.cs +++ b/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IHumanRepository.cs @@ -5,11 +5,10 @@ namespace Pgan.PoracleWebNet.Core.Abstractions.Repositories; public interface IHumanRepository { public Task> GetAllAsync(); - public Task GetByIdAsync(string id); + public Task GetByIdAsync(string id); public Task CreateAsync(Human human); public Task UpdateAsync(Human human); public Task> GetByIdsAsync(IEnumerable ids); public Task ExistsAsync(string id); - public Task DeleteAllAlarmsByUserAsync(string userId); public Task DeleteUserAsync(string userId); } diff --git a/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IOidcSessionRepository.cs b/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IOidcSessionRepository.cs index c388ffb9..3cfd2d4c 100644 --- a/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IOidcSessionRepository.cs +++ b/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IOidcSessionRepository.cs @@ -4,7 +4,8 @@ namespace Pgan.PoracleWebNet.Core.Abstractions.Repositories; /// /// Persistence for server-side OIDC refresh sessions (rotation families). All bulk revoke/cleanup -/// methods commit immediately via EF Core's set-based ExecuteUpdateAsync/ExecuteDeleteAsync. +/// methods commit immediately: the revokes via EF Core's set-based ExecuteUpdateAsync, the +/// cleanup via raw SQL because the aliased delete EF generates is invalid on MariaDB (see #707). /// public interface IOidcSessionRepository { @@ -28,6 +29,9 @@ public interface IOidcSessionRepository /// Revokes every still-active session for a user (admin disable / logout-everywhere). public Task RevokeAllForUserAsync(string userId, string reason); - /// Set-based delete of expired rows and revoked rows older than the retention window. + /// + /// Set-based delete of expired rows and revoked rows older than the retention window. + /// Returns the number of rows removed. + /// public Task DeleteExpiredAndStaleAsync(TimeSpan revokedRetention); } diff --git a/Core/Pgan.PoracleWebNet.Core.Repositories/HumanRepository.cs b/Core/Pgan.PoracleWebNet.Core.Repositories/HumanRepository.cs index 08f05bd5..021a88b3 100644 --- a/Core/Pgan.PoracleWebNet.Core.Repositories/HumanRepository.cs +++ b/Core/Pgan.PoracleWebNet.Core.Repositories/HumanRepository.cs @@ -73,19 +73,10 @@ public async Task UpdateAsync(Human human) public async Task ExistsAsync(string id) => await this._context.Humans.AnyAsync(h => h.Id == id); - public async Task DeleteAllAlarmsByUserAsync(string userId) - { - var count = 0; - count += await this._context.Monsters.Where(m => m.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Raids.Where(r => r.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Eggs.Where(e => e.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Quests.Where(q => q.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Invasions.Where(i => i.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Lures.Where(l => l.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Nests.Where(n => n.Id == userId).ExecuteDeleteAsync(); - count += await this._context.Gyms.Where(g => g.Id == userId).ExecuteDeleteAsync(); - return count; - } + // DeleteAllAlarmsByUserAsync lived here and was dead: HumanService has looped the tracking proxy + // since the PoracleNG migration, so nothing reached it. Its eight ExecuteDeleteAsync calls would + // each have emitted the aliased DELETE that MariaDB rejects (#707), so it could not have worked + // had anything called it. Alarm deletion belongs to PoracleNG, which reloads its own state. public async Task DeleteUserAsync(string userId) { diff --git a/Core/Pgan.PoracleWebNet.Core.Repositories/OidcSessionRepository.cs b/Core/Pgan.PoracleWebNet.Core.Repositories/OidcSessionRepository.cs index bb91c6dc..6dc0a1f0 100644 --- a/Core/Pgan.PoracleWebNet.Core.Repositories/OidcSessionRepository.cs +++ b/Core/Pgan.PoracleWebNet.Core.Repositories/OidcSessionRepository.cs @@ -61,9 +61,16 @@ public async Task DeleteExpiredAndStaleAsync(TimeSpan revokedRetention) { DateTime now = DateTime.UtcNow; DateTime revokedCutoff = now - revokedRetention; - return await this._context.OidcSessions - .Where(s => s.ExpiresAt < now || (s.RevokedAt != null && s.RevokedAt < revokedCutoff)) - .ExecuteDeleteAsync(); + + // Raw SQL rather than ExecuteDeleteAsync: MySql.EntityFrameworkCore emits the aliased + // single-table form -- DELETE FROM `oidc_sessions` AS `o` WHERE ... -- and MariaDB rejects + // that outright (1064; it wants the multi-table `DELETE o FROM ... AS o` when an alias is + // present). Every cleanup pass since the feature shipped threw and logged a warning, so the + // table only ever grew. Verified against MariaDB 10.8.2. Identifiers are left bare and + // unquoted so the same statement parses on MariaDB, MySQL and the SQLite the repository + // tests run against. See #707. + return await this._context.Database.ExecuteSqlInterpolatedAsync( + $"DELETE FROM oidc_sessions WHERE expires_at < {now} OR (revoked_at IS NOT NULL AND revoked_at < {revokedCutoff})"); } private static OidcSession ToModel(OidcSessionEntity e) => new() diff --git a/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerMeTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerMeTests.cs index 55f6416b..cba73db0 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerMeTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerMeTests.cs @@ -151,6 +151,62 @@ public async Task MeSignsOutAnAccountThatNoLongerExists() j => j.GenerateTokenWithReplacedProfile(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + /// + /// Blocking a user did nothing to an existing token, which kept full read/write access for the rest of + /// its 24-hour life. The SPA signs out on 401, so this is what ends the session. See #597. + /// + [Fact] + public async Task MeSignsOutABlockedAccount() + { + SetupUser(this._sut, profileNo: 1); + this._humanService.Setup(s => s.GetByIdAsync("123456789")) + .ReturnsAsync(new Human { CurrentProfileNo = 1, Enabled = 1, AdminDisable = 1 }); + + var result = await this._sut.Me(); + + Assert.IsType(result); + } + + /// + /// That 401 ends the CALLER's session, and while inspecting an account the caller is the admin -- so + /// inspecting a blocked user signed the admin out of their own session, with the SPA discarding the + /// stashed admin token along with it. A lapsed subscription is the main thing an admin inspects an + /// account to confirm. See #706. + /// + [Fact] + public async Task MeDoesNotSignOutAnAdminInspectingABlockedAccount() + { + SetupUser(this._sut, profileNo: 1); + ((ClaimsIdentity)this._sut.User.Identity!).AddClaim(new Claim("impersonatedBy", "999")); + this._humanService.Setup(s => s.GetByIdAsync("123456789")) + .ReturnsAsync(new Human { CurrentProfileNo = 1, Enabled = 1, AdminDisable = 1 }); + + var result = await this._sut.Me(); + + var userInfo = Assert.IsType(Assert.IsType(result).Value); + + // Returned as data, not as a refusal: the SPA renders its blocked banner from these two. + Assert.True(userInfo.AdminDisable); + Assert.False(userInfo.Enabled); + } + + /// + /// The deleted-account 401 is deliberately not relaxed for inspection: there is no account left to + /// render. The SPA drops the admin back to their own token rather than ending the session. See #706. + /// + [Fact] + public async Task MeStillSignsOutAnInspectionOfAnAccountThatNoLongerExists() + { + SetupUser(this._sut, profileNo: 1); + ((ClaimsIdentity)this._sut.User.Identity!).AddClaim(new Claim("impersonatedBy", "999")); + this._humanService.Setup(s => s.GetByIdAsync("123456789")) + .ReturnsAsync((Human?)null); + + var result = await this._sut.Me(); + + Assert.IsType(result); + } + /// /// The resync used to rebuild the token from UserInfo, which has no impersonatedBy field, so an admin /// impersonation session lost the only record of what it was - on exactly the out-of-band profile diff --git a/Tests/Pgan.PoracleWebNet.Tests/Repositories/NoAliasedDeleteTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Repositories/NoAliasedDeleteTests.cs new file mode 100644 index 00000000..ba5d6ffe --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Repositories/NoAliasedDeleteTests.cs @@ -0,0 +1,75 @@ +using System.Text.RegularExpressions; + +namespace Pgan.PoracleWebNet.Tests.Repositories; + +/// +/// ExecuteDeleteAsync is unusable against this deployment. MySql.EntityFrameworkCore emits the +/// aliased single-table form — DELETE FROM `t` AS `x` WHERE … — and MariaDB answers 1064; the +/// multi-table DELETE x FROM t AS x is required once an alias is present. Verified against +/// MariaDB 10.8.2. +/// +/// Nothing catches this at build time and nothing catches it in the test suite either, because the +/// repository tests run on SQLite, whose provider does not emit the alias. It reaches production green +/// and fails on every call. That is how the OIDC session cleanup shipped never having run once (#707), +/// and how QuickPickAppliedStateRepository acquired its load-and-remove workaround before it. +/// +/// Use raw SQL with unquoted identifiers, or load and RemoveRange when the row count is small. +/// ExecuteUpdateAsync is fine — MariaDB accepts UPDATE t AS x SET x.c = …. +/// +public sealed class NoAliasedDeleteTests +{ + private static readonly string[] ScannedProjects = + [ + "Core", + "Data", + "Applications/Pgan.PoracleWebNet.Api", + ]; + + // Matches the call, not prose: the surrounding comments and doc-comments name the method freely. + private static readonly Regex CallSite = new(@"\.ExecuteDeleteAsync\s*\(", RegexOptions.Compiled); + + [Fact] + public void NoProductionCodeCallsExecuteDeleteAsync() + { + var root = FindSolutionRoot(); + + var offenders = ScannedProjects + .Select(p => Path.Combine(root, p.Replace('/', Path.DirectorySeparatorChar))) + .Where(Directory.Exists) + .SelectMany(dir => Directory.EnumerateFiles(dir, "*.cs", SearchOption.AllDirectories)) + .Where(f => !IsBuildOutput(f, root)) + .SelectMany(f => File.ReadLines(f) + .Select((line, i) => (line, number: i + 1)) + .Where(x => CallSite.IsMatch(x.line)) + .Select(x => $"{Path.GetRelativePath(root, f)}:{x.number}")) + .OrderBy(x => x, StringComparer.Ordinal) + .ToList(); + + Assert.True( + offenders.Count == 0, + "ExecuteDeleteAsync is back: " + string.Join(", ", offenders) + + ". The provider emits DELETE FROM `t` AS `x`, which MariaDB rejects with a 1064 at runtime " + + "while the SQLite-backed tests stay green. Use raw SQL with unquoted identifiers, or load " + + "the rows and RemoveRange them. See #707."); + } + + private static bool IsBuildOutput(string file, string root) + { + var relative = Path.GetRelativePath(root, file); + var segments = relative.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + return segments.Any(s => s is "bin" or "obj"); + } + + private static string FindSolutionRoot() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + + while (dir != null && !File.Exists(Path.Combine(dir.FullName, "Pgan.PoracleWebNet.slnx"))) + { + dir = dir.Parent; + } + + Assert.NotNull(dir); + return dir.FullName; + } +} diff --git a/Tests/Pgan.PoracleWebNet.Tests/Repositories/OidcSessionRepositoryTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Repositories/OidcSessionRepositoryTests.cs index 14dcc07f..7c8f4a3e 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Repositories/OidcSessionRepositoryTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Repositories/OidcSessionRepositoryTests.cs @@ -1,5 +1,7 @@ +using System.Data.Common; using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; using Pgan.PoracleWebNet.Core.Models; using Pgan.PoracleWebNet.Core.Repositories; using Pgan.PoracleWebNet.Data; @@ -8,15 +10,16 @@ namespace Pgan.PoracleWebNet.Tests.Repositories; /// /// Repository tests over a real relational provider (SQLite in-memory) because the rotation guard -/// and cleanup use ExecuteUpdateAsync/ExecuteDeleteAsync, which the EF InMemory -/// provider cannot translate. Covers the retention semantics (decoupled revoked-row retention) and -/// the atomic rotation guard. +/// uses ExecuteUpdateAsync, which the EF InMemory provider cannot translate. Covers the +/// retention semantics (decoupled revoked-row retention), the atomic rotation guard, and the shape +/// of the emitted cleanup statement. /// public sealed class OidcSessionRepositoryTests : IDisposable { private readonly SqliteConnection _connection; private readonly PoracleWebContext _context; private readonly OidcSessionRepository _repo; + private readonly CommandCapture _commands = new(); public OidcSessionRepositoryTests() { @@ -24,6 +27,7 @@ public OidcSessionRepositoryTests() this._connection.Open(); var options = new DbContextOptionsBuilder() .UseSqlite(this._connection) + .AddInterceptors(this._commands) .Options; this._context = new PoracleWebContext(options); this._context.Database.EnsureCreated(); @@ -133,4 +137,58 @@ public async Task GetByHash_ReturnsMatchingSession_OrNull() Assert.NotNull(await this._repo.GetByHashAsync("known")); Assert.Null(await this._repo.GetByHashAsync("missing")); } + + /// + /// The retention test above passed for the whole time cleanup was broken in production: SQLite + /// accepts what EF generated, MariaDB answered 1064 to DELETE FROM `oidc_sessions` AS `o`, + /// and no test looked at the statement itself. This one does. See #707. + /// + [Fact] + public async Task DeleteExpiredAndStale_EmitsUnaliasedDelete_MariaDbCannotParseTheAliasedForm() + { + await this.SeedAsync("expired", "f1", expiresAt: DateTime.UtcNow.AddMinutes(-1)); + this._commands.Executed.Clear(); + + await this._repo.DeleteExpiredAndStaleAsync(TimeSpan.FromDays(2)); + + var delete = Assert.Single( + this._commands.Executed, + c => c.TrimStart().StartsWith("DELETE", StringComparison.OrdinalIgnoreCase)); + + Assert.DoesNotContain(" AS ", delete, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("`", delete, StringComparison.Ordinal); + } + + private sealed class CommandCapture : DbCommandInterceptor + { + public List Executed { get; } = []; + + public override InterceptionResult NonQueryExecuting( + DbCommand command, CommandEventData eventData, InterceptionResult result) + { + this.Executed.Add(command.CommandText); + return base.NonQueryExecuting(command, eventData, result); + } + + public override ValueTask> NonQueryExecutingAsync( + DbCommand command, CommandEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + this.Executed.Add(command.CommandText); + return base.NonQueryExecutingAsync(command, eventData, result, cancellationToken); + } + + public override InterceptionResult ReaderExecuting( + DbCommand command, CommandEventData eventData, InterceptionResult result) + { + this.Executed.Add(command.CommandText); + return base.ReaderExecuting(command, eventData, result); + } + + public override ValueTask> ReaderExecutingAsync( + DbCommand command, CommandEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + this.Executed.Add(command.CommandText); + return base.ReaderExecutingAsync(command, eventData, result, cancellationToken); + } + } } diff --git a/docs/configuration/oidc-refresh-tokens.md b/docs/configuration/oidc-refresh-tokens.md index b28b5ca8..61a4b748 100644 --- a/docs/configuration/oidc-refresh-tokens.md +++ b/docs/configuration/oidc-refresh-tokens.md @@ -140,7 +140,7 @@ session and one rotation chain. (atomic rotate / revoke) │ FamilyId chain, │ │ EncryptedRefreshToken │ OidcSessionCleanupService │ (DataProtection) │ - (~6h ExecuteDeleteAsync) └──────────────────────────┘ + (~6h set-based DELETE) └──────────────────────────┘ ``` ### Login and refresh-token issuance @@ -404,8 +404,10 @@ relies solely on `/token` (`grant_type=refresh_token`), `/userinfo`, and `expire Token hashing uses **SHA-256** over 32 random bytes with a unique index — a full-entropy secret correctly uses a fast hash (never bcrypt/PBKDF2) for O(1) indexed lookup. Expired and old-revoked -session rows are pruned by a background `OidcSessionCleanupService` (~every 6 hours, set-based -`ExecuteDeleteAsync`). +session rows are pruned by a background `OidcSessionCleanupService` (~every 6 hours, one set-based +`DELETE`). That delete is issued as raw SQL rather than EF's `ExecuteDeleteAsync`, which emits an +aliased `DELETE FROM oidc_sessions AS o` — valid on SQLite and MySQL 8, a 1064 syntax error on +MariaDB (#707). --- diff --git a/docs/poracleng-v2-review.md b/docs/poracleng-v2-review.md index 0a5e33f9..a4803ad4 100644 --- a/docs/poracleng-v2-review.md +++ b/docs/poracleng-v2-review.md @@ -50,7 +50,7 @@ Net consequence: **without three explicit asks to jfberry, the keystone deletion | 7 | `LocationController.UpdateLanguage` → `HumanService.UpdateAsync` → `HumanRepository.UpdateAsync` | Generic direct-DB human update for language | **YES** | `POST /api/v2/humans/{id}/language` exists — swap now | | 8 | `ProfileRepository` (all methods) + `ProfileService.Create/Update/Delete` | Dead code — controllers already use proxy | **YES** | Delete outright; v2 profile endpoints exist | | 9 | `HumanRepository.GetByIdAsync`/`ExistsAsync`/`CreateAsync` | Dead — service uses proxy | **YES** | Delete dead methods | -| 10 | `HumanRepository.DeleteAllAlarmsByUserAsync` | Dead — service loops via proxy | **PARTIAL** | Delete dead method now; one-shot purge endpoint is a nice-to-have (ask #6) | +| 10 | ~~`HumanRepository.DeleteAllAlarmsByUserAsync`~~ | Dead — service loops via proxy | **DONE** | Deleted in #707 (its `ExecuteDeleteAsync` calls could not run on MariaDB anyway); one-shot purge endpoint is still a nice-to-have (ask #6) | | 11 | `DashboardService.GetAllTrackingAsync` (proxy, not direct DB) | Fetches full payloads to count | **YES** | Snapshot/counts — perf win, not DB elimination | **Verdict:** rows 6–11 close on v2 (some are pure dead-code deletion). Rows 1–5 — the entire reason `PoracleContext` still exists — require **four new PoracleNG capabilities.** Until those land, `HumanRepository` shrinks to ~3 admin methods that still pin `PoracleContext`, and `UserAreaDualWriter` stays in full. @@ -123,7 +123,7 @@ The three **High** asks (trusted setAreas, admin list, batch resolve) are the ga |---|---|---| | 0a | Swap `LocationController.UpdateLanguage` to proxy `SetLanguageAsync` (→ `POST /api/v2/humans/{id}/language`). Removes the last live `HumanRepository.UpdateAsync` caller. | S | | 0b | Swap `UserGeofenceService:292` display-name read to proxy `GetHumanAsync`. | S | -| 0c | Delete dead `ProfileRepository`/`IProfileRepository` + `ProfileService` CRUD; dead `HumanRepository` methods (`GetByIdAsync`/`ExistsAsync`/`CreateAsync`/`DeleteAllAlarmsByUserAsync`) + `EnsureNotNullDefaults`. Update test mocks. Keep only `GetAllAsync`/`GetByIdsAsync`/`DeleteUserAsync` until v2 admin endpoints land. | M | +| 0c | Delete dead `ProfileRepository`/`IProfileRepository` + `ProfileService` CRUD; dead `HumanRepository` methods (`GetByIdAsync`/`ExistsAsync`/`CreateAsync`; `DeleteAllAlarmsByUserAsync` already gone in #707) + `EnsureNotNullDefaults`. Update test mocks. Keep only `GetAllAsync`/`GetByIdsAsync`/`DeleteUserAsync` until v2 admin endpoints land. | M | **Phase 1 — v2 read path (behind `Poracle:ApiVersion` flag):**