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.

Sign up for GitHub

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

Jump to bottom

/sessions endpoint returns sessions with complete identities even when configured AAL for whoami isn't reached #3671

Open
5 tasks done
K3das opened this issue Dec 22, 2023 · 3 comments · May be fixed by #4305
Open
5 tasks done

/sessions endpoint returns sessions with complete identities even when configured AAL for whoami isn't reached #3671

K3das opened this issue Dec 22, 2023 · 3 comments · May be fixed by #4305
Labels
bug Something is not working.

Comments

@K3das
Copy link
Contributor

K3das commented Dec 22, 2023

Preflight checklist

Ory Network Project

No response

Describe the bug

When required_aal for whoami is set as highest_available, Kratos will avoid leaking identity information without a valid AAL

kratos/selfservice/flow/login/hook.go

Lines 230 to 231 in 0c5ea9b

// If AAL is not satisfied, we omit the identity to preserve the user's privacy in case of a phishing attack.
response.Session.Identity = nil

kratos/session/handler.go

Lines 229 to 236 in 0c5ea9b

if err := h.r.SessionManager().DoesSessionSatisfy(r, s, c.SessionWhoAmIAAL(r.Context()),
// For the time being we want to update the AAL in the database if it is unset.
UpsertAAL,
); errors.As(err, &aalErr) {
h.r.Audit().WithRequest(r).WithError(err).Info("Session was found but AAL is not satisfied for calling this endpoint.")
h.r.Writer().WriteError(w, r, err)
return
} else if err != nil {

Unfortunately, this check is only performed on /sessions/whoami, allowing other /sessions requests without a higher AAL - leaking the user's identity.

Reproducing the bug

  1. Setup Kratos with whoami required_aal set to highest_available, and an identity that supports AAL2
  2. Login with AAL1
  3. Using the AAL1 session, request /sessions
  4. Previous sessions will have an identity field

Relevant log output

No response

Relevant configuration

session:
  whoami:
    required_aal: highest_available

Version

master, but could be reproduced on 1.0.0

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

No response
@K3das K3das added the bug Something is not working. label Dec 22, 2023
@alnr
Copy link
Contributor

alnr commented Jan 9, 2024

This is by design, since /sessions is an admin API. Please reopen if you think I'm mistaken.

@alnr alnr closed this as not planned Won't fix, can't repro, duplicate, stale Jan 9, 2024
@K3das
Copy link
Contributor Author

K3das commented Jan 9, 2024

It is not an admin API, and I don't believe I could reopen this issue myself.

https://www.ory.sh/docs/kratos/reference/api#tag/frontend/operation/listMySessions

@alnr alnr reopened this Jan 9, 2024
@aeneasr
Copy link
Member

aeneasr commented Feb 11, 2025

Great find! PRs welcomed :)

@K3das K3das linked a pull request Feb 12, 2025 that will close this issue
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something is not working.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: check aal on sessions list endpoint K3das/kratos
3 participants
@aeneasr @alnr @K3das