Altinn / altinn-notifications

Altinn platform microservice for handling notifications (mail, sms, etc)
MIT License
3 stars 3 forks source link

Authorization based filtering of recipients (#472) #520

Closed SandGrainOne closed 3 months ago

SandGrainOne commented 3 months ago

Description

Adding authorization logic to ensure that the recipients of notifications can access the resource they're being notified about.

My own thoughts is that we probably don't need to return a dictionary inside a dictionary. It might be enough with a list as the inner collection. The logic removes all results that are not Permit.

Related Issue(s)

Verification

Documentation

acn-sbuad commented 3 months ago

Placing the logic in the API project makes Core dependent on the Notifications project. I would rather at least the interface be defined in core. The service that will use the authorization is placed in core.

SandGrainOne commented 3 months ago

Placing the logic in the API project makes Core dependent on the Notifications project. I would rather at least the interface be defined in core. The service that will use the authorization is placed in core.

You're right. I forgot to move the interface. I don't want the service logic in Core because it would make Core dependent of PEP. Another idea is to move the logic to Integrations and rename it to "Client". That would isolate the external dependency to a more appropriate library.

acn-sbuad commented 3 months ago
System.InvalidOperationException : Unable to resolve service for type 'Microsoft.AspNetCore.Http.IHttpContextAccessor' while attempting to activate 'Altinn.Common.PEP.Clients.AuthorizationApiClient'.

Getting this error when plugging this code into the new logic in contact point service. Did you experience this when testing locally? Wondering if IHttpContextAccessor should be added to the AddAuthorizationService function.

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud