kohler / hotcrp

HotCRP conference review software
http://read.seas.harvard.edu/~kohler/hotcrp
Other
328 stars 111 forks source link

Extended oauth to support importing affiliations and group mappings #341

Closed ichdasich closed 5 months ago

ichdasich commented 5 months ago

This pull request extends the oauth2 functionality of hotcrp with a configurable group import and an import of the 'affiliation' property, if provided.

ichdasich commented 5 months ago

Changes should be in; Please take a look.

kohler commented 5 months ago

My thought is that this work only runs when a user logs in, so it's a little weird to remove permissions. If they just use an existing session their permissions will remain.

ichdasich commented 5 months ago

That is somewhat correct. But the issue there is the paradigm break in how oauth is implemented in hotcrp at the moment. Ideally, each request would be authenticated, i.e., the session would be handled with oauth.

I am usually using oauth via nginx, and had initially planned to implement it by making some patches to the http-auth mechanic; That would then also ensure that no stale session could be reused. However, that entails doing some more work in location/path auth requirements, i.e., some more work on the nginx conf, moving some application logic there. Similarly, with that model, perms could technically be put/sourced from the token/session.

Ultimately, the issue of authz only being updated extends to authn, i.e., a session can also just be reused if, e.g., the central account is locked (regardless of the authz level).

ichdasich commented 5 months ago

Ah. And please note that, at the moment, permission removal can only be configured, i.e., is not the default. ;-) Still, i'd argue that one would expect perms to (eventually) disappear if they are revoked centrally and have been granted from central.

kohler commented 5 months ago

Ideally, each request would be authenticated, i.e., the session would be handled with oauth.

I really don’t think OpenID Connect is supposed to be used on every website access. Are you sure that's the intended use case? I understood the point being to authenticate via a third-party provider at the start of a session. That’s how “sign in with Google” etc. works. OAuth for API access can involve access tokens, but HotCRP is a client-side application.

HotCRP does not, however, expire an OpenID session authentication on any specific timespan. I believe there's probably a way for an OAuth provider to say “this authentication is only good until time T and then must be revalidated.” But I'm not sure how this is expressed in terms of OAuth/OpenID claims—maybe you are. (In particular, the exp claim is typically around one hour or less in the future, and no website using “sign in with” expires a signin after one hour.)

ichdasich commented 5 months ago

Me neither; I usually have an nginx config as follows:

In the resource to protect:

                auth_request /internal-auth/oauth2/auth;

                error_page 401 = https://www.example.com/oauth2/start?rd=$scheme://www.example.com$request_uri;
                error_page 403 = https://www.example.com/oauth2/start?rd=$scheme://www.example.com$request_uri;

                auth_request_set $user   $upstream_http_x_auth_request_preferred_username;
                auth_request_set $email  $upstream_http_x_auth_request_email;
                auth_request_set $groups  $upstream_http_x_auth_request_groups;
                proxy_set_header X-User  $user;
                proxy_set_header X-Email $email;
                proxy_set_header X-Groups $groups;

                auth_request_set $token  $upstream_http_authorization;
                proxy_set_header X-Access-Token $token;

                auth_request_set $auth_cookie $upstream_http_set_cookie;
                add_header Set-Cookie $auth_cookie;

                auth_request_set $auth_cookie_name_upstream_1 $upstream_cookie_auth_cookie_name_1;

                if ($auth_cookie ~* "(; .*)") {
                        set $auth_cookie_name_0 $auth_cookie;
                        set $auth_cookie_name_1 "auth_cookie_name_1=$auth_cookie_name_upstream_1$1";
                }

                if ($auth_cookie_name_upstream_1) {
                        add_header Set-Cookie $auth_cookie_name_0;
                        add_header Set-Cookie $auth_cookie_name_1;
                }
  }              
  location /oauth2/ {
    proxy_set_header Host       $host;
    proxy_set_header X-Real-IP  $remote_addr;
    proxy_pass http://127.0.0.1:4180/oauth2/;
  }

  location /internal-auth/ {
    internal;
    proxy_set_header Host       $host;
    proxy_set_header X-Real-IP  $remote_addr;
    proxy_pass http://127.0.0.1:4180/;
  }

Together with https://github.com/oauth2-proxy/oauth2-proxy with a config similar to:

provider="keycloak-oidc"
client_id="client-identifier-or-name"
client_secret="<secret>"
redirect_url="https://www.example.com/oauth2/callback"
oidc_issuer_url="https://auth.example.com/realms/examplerealm"
email_domains="*"
http_address="127.0.0.1:4180"
cookie_secret="<cookiesecret>"
code_challenge_method="S256"
reverse_proxy="true"
scope="openid"
oidc_groups_claim="groups"
real_client_ip_header="X-Real-IP"
whitelist_domains="*.example.com"
set_authorization_header="true"
set_xauthrequest="true"

This then indeed auth'es every request vs. oauth2-proxy, which in turn takes care of, e.g., retiring an oauth2-session if the provider terminates it.

The plus part of this solution is:

As i said, this was what i initially thought would make sense just sailing on the http_auth functionality; However, I then found the openid implementation in hotcrp and decided to just try extending that.

kohler commented 5 months ago

OK, I see!

Looks good. You agree to release copyright according to LICENSE?

ichdasich commented 5 months ago

Sure, anything MIT-like is fine for me. ;-)