travisghansen / external-auth-server

easy auth for reverse proxies
MIT License
330 stars 44 forks source link

GitHub Oauth Permission Write? #166

Closed autarchprinceps closed 1 year ago

autarchprinceps commented 1 year ago

Using EAS with GitHub Oauth requests "Update all user data". Shouldn't read access at the most be enough?

travisghansen commented 1 year ago

That’s generally setup when you create the client right? I don’t think eas really cares what the associated permissions are. If you are leveraging the github userinfo bit then the scopes should include enough permissions to invoke the enabled endpoints. They are all GET requests though.

https://github.com/travisghansen/external-auth-server/blob/master/src/plugin/oauth/index.js#L2371

travisghansen commented 1 year ago

Also what did you put in the scopes parameter of the plugin config?

autarchprinceps commented 1 year ago

We used the contrib/generate-config-helm-traefik-github.js script. As far as I see the scopes there is: scopes: ["user"] userinfo: { provider: "github", config: { fetch_teams: true, fetch_organizations: true, fetch_emails: true } } and then a read only assertion whether the github team ID is one of those we provided. Nothing that jumps out to me as being write access. It definitively isn't configured in the GitHub Oauth App either. There is no option to set there concerning such permission.

travisghansen commented 1 year ago

Ah, so that’s the issue. Scopes are available here: https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps

I would start with those 3 and see if everything works.

autarchprinceps commented 1 year ago

I see, we'll test this at some point, hopefully this week. If we get a working version of the contrib script with less permission, I'll open up a PR for the script.

travisghansen commented 1 year ago

Great!

autarchprinceps commented 1 year ago

Yeah, that works fine. I can still access Ingress with those scopes, and I get read only displayed now.

image

https://github.com/travisghansen/external-auth-server/pull/167

travisghansen commented 1 year ago

Can you share the assertions you have on the userinfo?

autarchprinceps commented 1 year ago

That part didn't need to change. We primarily assert against team membership, which can include multiple teams, incl. different organisations, and MFA.

{
assertions: {
    /**
     * assert the token(s) has not expired
     */
    exp: true,
    userinfo: [
      {
        query_engine: "jp",
        query: "$.teams[*].id",
        rule: {
          method: "contains-any",
          value: github_team_ids
          //negate: true,
          //case_insensitive: true
        }
      },
      {
        query_engine: "jp",
        query: "$.two_factor_authentication",
        rule: {
            method: "eq",
            value: true,
            //negate: true,
            //case_insensitive: true
        }
      }
    ]
  }
}
autarchprinceps commented 1 year ago

We completely use the public script version, and fill it with a .env with the parameters mentioned on the top of the script for different environments. Aside from those GitHub secrets obviously, there is nothing we are keeping hidden.

travisghansen commented 1 year ago

Yup! Just wanted to confirm indeed the userinfo data is still being populated correctly with the more restrictive set of permissions.

If it's your first time using the project I'm interested in general feedback from your experience thus far!

runningman84 commented 1 year ago

@travisghansen we have seen this behavior recently: eas_auth Why is it still requesting "update all user data"? The configuration was created using this script: https://github.com/travisghansen/external-auth-server/blob/master/contrib/generate-config-helm-traefik-github.js

travisghansen commented 1 year ago

I’m not super familiar with all the github scopes etc unfortunately. Are there some default scopes set for the client id/secret that may be causing something like that?