mindersec / minder / 33905986826

62%

push

github

Fix panic recovery logging and OAuth callback race (#6733)

* Emit the panic recovery log event

The gRPC recovery handler built a zerolog event with the panic value
and stack trace but never called a terminator, so zerolog discarded
the event entirely and recovered panics were invisible in the logs.
Move the message into Msg() so the event is actually emitted.

* Retry provider creation on unique violation in OAuth callback

When two OAuth callbacks for the same provider raced, the loser got a
unique violation from CreateProviderFromSessionState along with a nil
provider. The handler logged "Provider already exists" and then
dereferenced the nil provider, panicking the callback. The access
token from the losing flow was also never stored.

Retry the call once when the unique violation surfaces: the provider
now exists, so the retry takes the fetch path, stores this flow's
access token, and returns the existing provider. Add a regression
test that reproduces the race through the store mocks; it panics
without this fix.

* Return existing provider with uniqueness error on lost race

Review feedback on the retry approach: it re-read the session state and
stored a second access token, both unnecessary in the enrollment race.
Change the CreateProviderFromSessionState contract instead: when
creation loses the race, fetch the winner's provider and return it
alongside the uniqueness error, so the OAuth callback falls through
with no extra writes. Also switch the not-found check to the
generics-based errors.AsType.

22 of 24 new or added lines in 3 files covered. (91.67%)

21636 of 34986 relevant lines covered (61.84%)

39.45 hits per line