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

Allow Session change without session token #7883

Open
2 tasks
livio-a opened this issue May 1, 2024 · 3 comments · May be fixed by #7963
Open
2 tasks

Allow Session change without session token #7883

livio-a opened this issue May 1, 2024 · 3 comments · May be fixed by #7963
Assignees

Comments

@livio-a
Copy link
Member

livio-a commented May 1, 2024

The session update and deletion require the current session token as argument.
Since this adds extra complexity but no real additional security and prevents case like magic links, we want to remove this requirement.

We still require the session token on other resouces / endpoints, e.g. for finalizing the auth request or on idp intents.

Acceptance criteria

  • Deprecate the token field on update and delete endpoint
  • Update all documentation examples without token use
@livio-a
Copy link
Member Author

livio-a commented May 1, 2024

relates to #6099

@muhlemmer
Copy link
Contributor

@livio-a, I have 2 questions:

  1. If we remove the token from the delete endpoint, it is a breaking change. Do we really want this? The token was already optional.

Sessions can be terminated by either:
- the authenticated user
- a manager, who is granted `session.delete` (e.g. ORG_OWNER) on the authenticated users organisation
- providing the current session_token in the body.

As without the token we can't establish the "authenticated user" and the logic falls back to the session.delete permission. I kind of figured because the OIDC integration tests break, as the CTXLOGIN user does not have the permission through the ORG_USER_MANAGER role for example.

I have a draft PR here if you want to check what I mean: #7963

  1. Did we also say we no longer want to update the session token on write? I'm not sure anymore.

@livio-a
Copy link
Member Author

livio-a commented May 17, 2024

@livio-a, I have 2 questions:

  1. If we remove the token from the delete endpoint, it is a breaking change. Do we really want this? The token was already optional.

Sessions can be terminated by either:
- the authenticated user
- a manager, who is granted `session.delete` (e.g. ORG_OWNER) on the authenticated users organisation
- providing the current session_token in the body.

As without the token we can't establish the "authenticated user" and the logic falls back to the session.delete permission. I kind of figured because the OIDC integration tests break, as the CTXLOGIN user does not have the permission through the ORG_USER_MANAGER role for example.

I have a draft PR here if you want to check what I mean: #7963

  1. Did we also say we no longer want to update the session token on write? I'm not sure anymore.
  1. I would keep the token there as is (optional) and only change the update request.
  2. Always update and return the session token.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗 In progress
Development

Successfully merging a pull request may close this issue.

2 participants