Altinn / Altinn/altinn-authentication

Make IPartiesClient/IAccessManagementClient return types nullable + handle null in 22 callers (#488 follow-up)

Open
#2,109 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
8
Forks
6
Avg merge
2d 21h
Merged PRs (30d)
21

Description

## Summary

To finish the #488 nullable ratchet for the **Integration** project (and lock it with `TreatWarningsAsErrors`), three client interface methods that genuinely return `null` need their contracts made nullable — a change that ripples to their callers and so was deferred out of #2108 for review.

## What needs changing

- **`IPartiesClient`** — `GetPartyAsync`, `GetPartyByOrgNo`, `GetPartyByUuId` are typed `Task` but return `null` on non-OK responses. Make them `Task` (impl already returns null).
- **`IAccessManagementClient`** — `GetAccessPackage`: the implementation already returns `Task`, but the interface declares `Task` (a `CS8613` mismatch). Align the interface to `Task`.

## Caller impact (the reason this is separate)

All **22 production callers** of the party methods do `Party party = await _partiesClient.GetPartyByOrgNo(...)` — assigning to a **non-nullable** local and then dereferencing without a null check:

- `AuthenticationController` (1), `SystemUserClientDelegationController` (5)
- `ChangeRequestSystemUserService` (4), `RequestSystemUserService` (4), `SystemUserService` (8)

Making the return nullable surfaces a `CS8600`/`CS8602` at each — i.e. **latent NRE risk that exists today**. Each site needs a real decision: what should happen when the party isn't found? (return a Problem, throw, skip, etc.) This is the "don't change a shared contract without reviewing all downstream callers" guidance from prior review.

## Tasks

- [ ] Make `IPartiesClient.GetParty*` return `Task` and `IAccessManagementClient.GetAccessPackage` return `Task`.
- [ ] Review each of the 22 party callers and add appropriate null handling (not just `!`).
- [ ] Enable `` on the Integration project (it will then be at 0 warnings) — completing its #488 ratchet.

## Context
- Follow-up to #2108 (Integration 60→7 warnings).
- Part of #488.
- Related: the `HandleResponse` 403/500 issue #2101 (same client).

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the IPartiesClient and IAccessManagementClient declarations and their existing implementations, then inspect the 22 callers named in AuthenticationController, SystemUserClientDelegationController, ChangeRequestSystemUserService, RequestSystemUserService, and SystemUserService. Decide and implement explicit null handling at each call site, then enable TreatWarningsAsErrors in the Integration project and verify the build has zero warnings.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
authentication, backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.