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

Use reva token for auth #9245

Closed
tbsbdr opened this issue Jun 15, 2023 · 12 comments · Fixed by #9871
Closed

Use reva token for auth #9245

tbsbdr opened this issue Jun 15, 2023 · 12 comments · Fixed by #9871

Comments

@tbsbdr
Copy link
Contributor

tbsbdr commented Jun 15, 2023

Description

User Stories

  • As an admin I want to control the session lifetime in the client so that I'm not dependent on the IAM.

Value

Acceptance Criteria

Definition of ready

[ ] everybody needs to understand the value written in the user story
[ ] acceptance criteria has to be defined
[ ] all dependencies of the user story need to be identified
[ ] feature should be seen from an end user perspective
[ ] user story has to be estimated
[ ] story points need to be less then 20

Definition of done

  • Functional requirements
    [ ] functionality described in the user story works
    [ ] acceptance criteria are fulfilled
  • Quality
    [ ] code review happened
    [ ] CI is green
    [ ] critical code received unit tests by the developer
    [ ] automated tests passed (if automated tests are not available, this test needs to be created and passed
  • Non-functional requirements
    [ ] no sonar cloud issues

realtes to 9c37d8c

@dragotin
Copy link
Contributor

Proposal how to implement:

The extension system allows to register custom code in a post-sign-in-hook. There, the CERN code could be added to an extension that pulls the reva token and stores it in the browser.

Benefit for the product would for example be that an extension could be written that lists all logged in users on a dashboard. For that, the custom extension code would call an dashboard endpoint with the user name that just signed in.

@tbsbdr
Copy link
Contributor Author

tbsbdr commented Jun 26, 2023

Note: @labkode mentioned that Mattermost provides an option to control the lifetime via the client.

@tbsbdr tbsbdr changed the title Control session lifetime Control session lifetime via Client Jun 28, 2023
@tbsbdr
Copy link
Contributor Author

tbsbdr commented Jun 28, 2023

@DeepDiver1975 this patch 9c37d8c from CERN uses the REVA token so that the session lifetime can be controlled via the client. Can you assess if this option would be a security issue for the product? (Generally we wonder if this would be an option for the product.). thank you!

@DeepDiver1975
Copy link
Member

In a JWT based authentication mechanism the session lifetime is basically the lifetime of the JWT.

Assuming an OpenID Connect/OAuth setup this will result in following scenarios:

  • JWT expired -> go back to IdP and reauthenticate
  • JWT valid, server session expired -> go back to IdP - user still has a valid session within the IdP -> immediate redirect to the application

the scenario is only reasonable in case the IdP is setup to not reuse existing authenticated session and the application (owncloud web) is not setup to auto redirect to the IdP on the login screen.

I can not see any issues from a security perspective ....

let me know if you anything else

@tbsbdr
Copy link
Contributor Author

tbsbdr commented Jun 28, 2023

Thx! Fyi @hodyroff @labkode

@kulmann
Copy link
Member

kulmann commented Jun 28, 2023

Let me rephrase @tbsbdr question:

The web patch uses a kind of post login hook to use the just freshly created access token from the IdP to make an authenticated request to a reva endpoint which then generates a custom token (session life time is still controlled by the backend). This custom token is then used instead of the original access token in all following requests from ownCloud Web. The original access token is being sent into nirvana.

Please check if you need to reevaluate based on this @DeepDiver1975 :)

Regardless of the outcome, the possibility of a post login hook via extension system is a good idea.

@DeepDiver1975
Copy link
Member

That is an interesting detail which I need to think about.
Does this mean that the refresh token is also thrown away?

The functional impact needs to be thought about.

Overall my comment from about remains.....

@kulmann
Copy link
Member

kulmann commented Jun 28, 2023

Does this mean that the refresh token is also thrown away?

The particular setup doesn't allow to request the offline_access scope for web from what I know. So there is no refresh token that could be thrown away.

Some context again: The particular SSO team doesn't allow a session lifetime (neither through a long IdP session timeout nor via a refresh token) that is long enough to make their users happy, so engineering found an alternative solution.

@DeepDiver1975
Copy link
Member

Does that mean tokens have no expiry in this environment? Anyway ....

@kulmann
Copy link
Member

kulmann commented Jun 28, 2023

Does that mean tokens have no expiry in this environment? Anyway ....

No idea how the tokens behave client side. I just know the flow how to request such a custom token. You can ping @tbsbdr if you want to take a look at the requests in the browser, he has a test user.

@labkode
Copy link

labkode commented Jun 29, 2023

Hi,

Any decent web application will have session control independently of authentication source (OIDC, basic auth, LDAP, ...).

Here you can find two examples:

https://github.com/mattermost/mattermost/blob/c6916c7047507eb6915e2ed047d3e3688cf3694a/server/channels/app/oauth.go#L396

    session.AddProp(model.SessionPropOs, "OAuth2")
    session.AddProp(model.SessionPropBrowser, "OAuth2")

    session, err := a.Srv().Store().Session().Save(session)
    if err != nil {
        return nil, model.NewAppError("newSession", "api.oauth.get_access_token.internal_session.app_error", nil, "", http.StatusInternalServerError)
    }

indico/indico/indico/modules/auth/init.py

        identity.data = identity_info.data
    if user.is_blocked:
        raise MultipassException(_('Your Indico profile has been blocked.'))
    login_user(user, identity)


def login_user(user, identity=None, admin_impersonation=False):

@tbsbdr tbsbdr added this to the CERN Web Merge milestone Jul 10, 2023
@tbsbdr tbsbdr changed the title Control session lifetime via Client Use reva token for auth Jul 14, 2023
@JammingBen
Copy link
Contributor

Implemented via #9871.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants