Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Recurring MacaroonInitException: Must supply serialized macaroon. in sentry #12707

Closed
DMRobertson opened this issue May 11, 2022 · 2 comments · Fixed by #12723
Closed

Recurring MacaroonInitException: Must supply serialized macaroon. in sentry #12707

DMRobertson opened this issue May 11, 2022 · 2 comments · Fixed by #12723
Assignees
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@DMRobertson
Copy link
Contributor

Internal users: https://sentry.matrix.org/sentry/synapse-matrixorg/issues/219508/?query=is%3Aunresolved

First seen in 1.32.2, according to sentry.

Invalid session for OIDC callback

MacaroonInitException: Must supply serialized macaroon.
  File "synapse/handlers/oidc.py", line 220, in handle_oidc_callback
    session, state
  File "synapse/handlers/oidc.py", line 1186, in verify_oidc_session_token
    macaroon = pymacaroons.Macaroon.deserialize(session)
  File "pymacaroons/macaroon.py", line 50, in deserialize
    'Must supply serialized macaroon.'

Judging from the source code below, it looks like these are errors that we expect to encounter from time to time. Maybe logger.execption -> logger.warning?

# Deserialize the session token and verify it.
try:
session_data = self._token_generator.verify_oidc_session_token(
session, state
)
except (MacaroonInitException, MacaroonDeserializationException, KeyError) as e:
logger.exception("Invalid session for OIDC callback")
self._sso_handler.render_error(request, "invalid_session", str(e))
return
except MacaroonInvalidSignatureException as e:
logger.exception("Could not verify session for OIDC callback")
self._sso_handler.render_error(request, "mismatching_session", str(e))
return

@DMRobertson DMRobertson added A-Logging Synapse's logs (structured or otherwise). Not metrics. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. X-Needs-Discussion labels May 11, 2022
@DMRobertson
Copy link
Contributor Author

Discussed this during triage. Consensus: this is sensible, let's do it.

@DMRobertson DMRobertson self-assigned this May 12, 2022
DMRobertson pushed a commit that referenced this issue May 12, 2022
This means we won't get tracebacks reported to Sentry.

There are other `logger.exception` and `logger.error` calls in this
file. I tried to only target errors that

- show up a lot in Sentry
- don't sound like our fault
- are still reported to the end-user by other means.

Closes #12707.
@DMRobertson
Copy link
Contributor Author

I raised #12782 with extra info from sandhose from the linked PR. I'd actually forgotten about this issue; I think it's best to close.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant