From f39af3112391a6d6d6df42e6e5cc6d8115c07fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Wed, 3 Apr 2024 14:31:53 +0200 Subject: [PATCH] fix: Require read action on elements without auth attr, replace unauthorized URLs (#574) Also change "contains" to equality check when matching resources --- .../Common/Authorization/Constants.cs | 1 + .../Dialogs/Queries/Get/GetDialogQuery.cs | 45 ++++++++++---- tests/k6/tests/enduser/all-tests.js | 2 + tests/k6/tests/enduser/dialogDetails.js | 60 +++++++++++++++++++ .../serviceowner/testdata/01-create-dialog.js | 15 ++++- 5 files changed, 112 insertions(+), 11 deletions(-) create mode 100644 tests/k6/tests/enduser/dialogDetails.js diff --git a/src/Digdir.Domain.Dialogporten.Application/Common/Authorization/Constants.cs b/src/Digdir.Domain.Dialogporten.Application/Common/Authorization/Constants.cs index 000622f70..2c166532e 100644 --- a/src/Digdir.Domain.Dialogporten.Application/Common/Authorization/Constants.cs +++ b/src/Digdir.Domain.Dialogporten.Application/Common/Authorization/Constants.cs @@ -5,4 +5,5 @@ public static class Constants public const string MainResource = "main"; public const string ReadAction = "read"; public const string ElementReadAction = "elementread"; + public static readonly Uri UnauthorizedUri = new("urn:dialogporten:unauthorized"); } diff --git a/src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs b/src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs index bfbab5ba4..895c47b46 100644 --- a/src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs +++ b/src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs @@ -126,18 +126,20 @@ public async Task Handle(GetDialogQuery request, CancellationTo ); DecorateWithAuthorization(dto, authorizationResult); + ReplaceUnauthorizedUrls(dto); return dto; } - private static void DecorateWithAuthorization(GetDialogDto dto, DialogDetailsAuthorizationResult authorizationResult) + private static void DecorateWithAuthorization(GetDialogDto dto, + DialogDetailsAuthorizationResult authorizationResult) { - foreach (var (action, resources) in authorizationResult.AuthorizedAltinnActions) + foreach (var (action, resource) in authorizationResult.AuthorizedAltinnActions) { foreach (var apiAction in dto.ApiActions.Where(a => a.Action == action)) { - if ((apiAction.AuthorizationAttribute is null && resources.Contains(Constants.MainResource)) - || (apiAction.AuthorizationAttribute is not null && resources.Contains(apiAction.AuthorizationAttribute))) + if ((apiAction.AuthorizationAttribute is null && resource == Constants.MainResource) + || (apiAction.AuthorizationAttribute is not null && resource == apiAction.AuthorizationAttribute)) { apiAction.IsAuthorized = true; } @@ -145,8 +147,8 @@ private static void DecorateWithAuthorization(GetDialogDto dto, DialogDetailsAut foreach (var guiAction in dto.GuiActions.Where(a => a.Action == action)) { - if ((guiAction.AuthorizationAttribute is null && resources.Contains(Constants.MainResource)) - || (guiAction.AuthorizationAttribute is not null && resources.Contains(guiAction.AuthorizationAttribute))) + if ((guiAction.AuthorizationAttribute is null && resource == Constants.MainResource) + || (guiAction.AuthorizationAttribute is not null && resource == guiAction.AuthorizationAttribute)) { guiAction.IsAuthorized = true; } @@ -154,13 +156,36 @@ private static void DecorateWithAuthorization(GetDialogDto dto, DialogDetailsAut // Simple "read" on the main resource will give access to a dialog element, unless a authorization attribute is set, // in which case an "elementread" action is required - foreach (var dialogElement in dto.Elements.Where( - dialogElement => (dialogElement.AuthorizationAttribute is null) - || (dialogElement.AuthorizationAttribute is not null - && action == Constants.ElementReadAction))) + foreach (var dialogElement in dto.Elements.Where(dialogElement => (dialogElement.AuthorizationAttribute is null && action == Constants.ReadAction) + || (dialogElement.AuthorizationAttribute is not null && action == Constants.ElementReadAction))) { dialogElement.IsAuthorized = true; } } } + + private static void ReplaceUnauthorizedUrls(GetDialogDto dto) + { + // For all API and GUI actions and dialogelements where isAuthorized is false, replace the URLs with Constants.UnauthorizedUrl + foreach (var guiAction in dto.GuiActions.Where(a => !a.IsAuthorized)) + { + guiAction.Url = Constants.UnauthorizedUri; + } + + foreach (var apiAction in dto.ApiActions.Where(a => !a.IsAuthorized)) + { + foreach (var endpoint in apiAction.Endpoints) + { + endpoint.Url = Constants.UnauthorizedUri; + } + } + + foreach (var dialogElement in dto.Elements.Where(e => !e.IsAuthorized)) + { + foreach (var url in dialogElement.Urls) + { + url.Url = Constants.UnauthorizedUri; + } + } + } } diff --git a/tests/k6/tests/enduser/all-tests.js b/tests/k6/tests/enduser/all-tests.js index 9b75077f2..e97c98028 100644 --- a/tests/k6/tests/enduser/all-tests.js +++ b/tests/k6/tests/enduser/all-tests.js @@ -1,6 +1,8 @@ // This file is generated, see "scripts" directory +import { default as dialogDetails } from './dialogDetails.js'; import { default as dialogSearch } from './dialogSearch.js'; export default function() { + dialogDetails(); dialogSearch(); } diff --git a/tests/k6/tests/enduser/dialogDetails.js b/tests/k6/tests/enduser/dialogDetails.js new file mode 100644 index 000000000..87b7759c6 --- /dev/null +++ b/tests/k6/tests/enduser/dialogDetails.js @@ -0,0 +1,60 @@ +import { + describe, expect, expectStatusFor, + getEU, + setVisibleFrom, + postSO, + purgeSO } from '../../common/testimports.js' + +import { default as dialogToInsert } from '../serviceowner/testdata/01-create-dialog.js'; +export default function () { + + let dialogId = null; + let dialog = null; + + describe('Arrange: Create a dialog to test against', () => { + let d = dialogToInsert(); + setVisibleFrom(d, null); + let r = postSO("dialogs", d); + expectStatusFor(r).to.equal(201); + dialogId = r.json(); + }); + + describe('Perform simple dialog get', () => { + let r = getEU('dialogs/' + dialogId); + expectStatusFor(r).to.equal(200); + expect(r, 'response').to.have.validJsonBody(); + expect(r.json(), 'response json').to.have.property("id").to.equal(dialogId); + + dialog = r.json(); + }); + + describe('Check that authorized actions have real URLs', () => { + if (dialog == null) return; + expect(dialog, 'dialog').to.have.property("guiActions").with.lengthOf(1); + expect(dialog.guiActions[0], 'gui action').to.have.property("isAuthorized").to.equal(true); + expect(dialog.guiActions[0], 'url').to.have.property("url").to.include("https://"); + }); + + describe('Check that unauthorized actions to have default URLs', () => { + if (dialog == null) return; + expect(dialog, 'dialog').to.have.property("apiActions").with.lengthOf(1); + expect(dialog.apiActions[0], 'api action').to.have.property("isAuthorized").to.equal(false); + expect(dialog.apiActions[0], 'url').to.have.property("endpoints"); + for (let i=0; i { + if (dialog == null) return; + let r = purgeSO("dialogs/" + dialogId); + expect(r.status, 'response status').to.equal(204); + }); + + describe("Check if we get 404 Not found", () => { + if (dialog == null) return; + let r = getEU('dialogs/' + dialogId); + expectStatusFor(r).to.equal(404); + }); +} \ No newline at end of file diff --git a/tests/k6/tests/serviceowner/testdata/01-create-dialog.js b/tests/k6/tests/serviceowner/testdata/01-create-dialog.js index 3409c1550..d47a7bc84 100644 --- a/tests/k6/tests/serviceowner/testdata/01-create-dialog.js +++ b/tests/k6/tests/serviceowner/testdata/01-create-dialog.js @@ -22,9 +22,22 @@ export default function () { { "type": "Summary", "value": [ { "cultureCode": "nb_NO", "value": "Et sammendrag her. Maks 200 tegn, ingen HTML-støtte. Påkrevd. Vises i liste." } ] }, { "type": "AdditionalInfo", "value": [ { "cultureCode": "nb_NO", "value": "Utvidet forklaring (enkel HTML-støtte, inntil 1023 tegn). Ikke påkrevd. Vises kun i detaljvisning." } ] } ], + "guiActions": [ + { + "action": "read", + "url": "https://digdir.no", + "priority": "Primary", + "title": [ + { + "value": "Gå til dialog", + "cultureCode": "nb-no" + } + ] + } + ], "apiActions": [ { - "action": "some:action", + "action": "some_unauthorized_action", "dialogElementId": dialogElementId, "endPoints": [ {