Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caching of system users authorization details are broken #1363

Closed
elsand opened this issue Oct 31, 2024 · 1 comment
Closed

Caching of system users authorization details are broken #1363

elsand opened this issue Oct 31, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@elsand
Copy link
Member

elsand commented Oct 31, 2024

Description

When generating cache keys for system users, the system user id is not part of the list of identifiable claims on which the key is based

Reproduction

  1. Perform a request to a dialog, using a system user that has been granted access to the dialog, owned by the same organization (consumer/supplier)
  2. Perform a request to a dialog, using a different system user that has NOT been granted access to the dialog, owned by the same organization (consumer/supplier)

Expected behavior

  1. OK
  2. Unauthorized

Actual behavior

  1. OK
  2. OK

Additional information

The two requests will have identical cache keys, causing the second request from to hit the cache for the PDP call. This is a security issue, but mitigated by the fact that it must be owned by the same organization.

@elsand elsand added the bug Something isn't working label Oct 31, 2024
@elsand elsand self-assigned this Oct 31, 2024
elsand added a commit that referenced this issue Oct 31, 2024
## Description

This adds a check to include the system user id in the list of
identifiable claims, which is in turn used to generate a cache key for
authorization requests on dialog details accesses.

## Related Issue(s)

- #1363

## Verification

- [x] **Your** code builds clean without any errors or warnings
- [x] Manual testing done (required)
- [x] Relevant automated test added (if you find this hard, leave it and
we'll help out)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced methods to simplify the retrieval of system user IDs from
claims.
- Enhanced claims processing to include system user identifiers from
authorization details.

- **Bug Fixes**
- Streamlined logic in handling user ID extraction, improving
efficiency.

- **Tests**
- Added a test to verify the correct extraction of system user
identifiers from claims.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@elsand elsand closed this as completed Jan 8, 2025
@LeifHelstad
Copy link

LeifHelstad commented Jan 28, 2025

Test

For læringens del ønsker jeg å få til å teste #1363
Dessverre får jeg ikke helt tak i steg 2 i reproduksjonen. Hva er en "...different system user...owned by the same organization...".
Har noen mulighet til å forklare meg hva slags brukere jeg må benytte her?

Jeg har for eksempel testet tidligere at DAGL og REGN for samme org kan lese samme dialog slik at jeg får flere "lest av" på en og samme dialog. Jeg regner med at DAGL er fin å benytte for steg 1. men da at REGN er uegnet for steg 2. for da skal bruker egentlig ikke ha tilgang. Hva slags knytning skal eventuelt bruker i steg 2. ha til samme org som i 1.?

Test av denne blir mest for læring

Antakelig er denne testet godt nok, særlig siden den er såpass spesifikk.

Verdien av å teste denne vil ligge mer i forstå systembruker og autentisering ved å følge oppskriften fra Bjørn her:

Systembrukere kan anses som "virtuelle personer", som kan kan opprettes og delegeres rettigheter hos en organisasjon. Se https://docs.altinn.studio/authentication/what-do-you-get/systemuser/. For å teste denne vil man trenge en DAGL for å opprette to slike systembrukere. Den ene gis en rolle/tjenesterettighet, som gir tilgang til en eller flere dialoger for den tilhørende organisasjonen. Den andre gis ingen rettigheter. Ved hjelp av tokengeneratoren kan man autentisere seg som systembruker. Testen blir altså å først autentisere seg som systembruker nr 1 (med rettigheter), og forsøke å hente ut en liste med dialoger, som da skal gi et ikke-tomt resultat. Påfølgende identiske kall skal da treffe en autorisasjonscache, som gjør kallet raskere. Forsøk så å autentisere som systembruker nr 2 (den uten rettigheter), og gjør samme kall. Dette skal da returnere et tomt resultat. (Før fiks ville man her truffet cachen som tilhørte den første systembrukern og fått ut samme lista)

Lite å bekrefte

Siden feilen allerede er rettet er det heller ingen mulighet for å sette opp et feil-scenario å teste først for å se om man følger rett oppskrift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

2 participants