Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

*: add an OpenID Connect provider #389

Merged
merged 1 commit into from
Sep 10, 2017
Merged

Conversation

ericchiang
Copy link
Contributor

@ericchiang ericchiang commented May 9, 2017

Takes over from #165

Closes #332

cc @philips @gtaylor

See the README for usage with Dex or any other OIDC provider.

To test run a backend:

python3 -m http.server

Run dex and modify the example config with the oauth2 callback.

go get github.com/coreos/dex/cmd/dex
cd $GOPATH/src/github.com/coreos/dex
sed -i.bak \
  's|http://127.0.0.1:5555/callback|http://127.0.0.1:5555/oauth2/callback|g' \
   examples/config-dev.yaml
make
./bin/dex serve examples/config-dev.yaml

Then run the oauth2_proxy:

oauth2_proxy \
  --oidc-issuer-url http://127.0.0.1:5556/dex \
  --upstream http://localhost:8000 \
  --client-id example-app \
  --client-secret ZXhhbXBsZS1hcHAtc2VjcmV0 \
  --cookie-secret foo \
  --email-domain '*' \
  --http-address http://127.0.0.1:5555 \
  --redirect-url http://127.0.0.1:5555/oauth2/callback \
  --cookie-secure=false

Login with the username/password admin@example.com:password

Copy link

@gtaylor gtaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not yet familiar enough with oauth2_proxy guts to review most of this, but had one doc-related comment above!

README.md Outdated
2. Setup oauth2_proxy with the correct provider and using the default ports and callbacks.
3. Login with the fixture use in the dex guide.

-provider oidc
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be more explicitly marked as oauth2_proxy args? It makes sense if you've played with oauth2_proxy before, but may help those who are early in their evaluation/have yet to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README section was just copied over from the previous PR. Let me revamp it a bit. Thanks.

@@ -137,6 +141,22 @@ func (o *Options) Validate() error {
msgs = append(msgs, "missing setting for email validation: email-domain or authenticated-emails-file required.\n use email-domain=* to authorize all email addresses")
}

if o.OIDCIssuerURL != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, needs to come after overriding http.DefaultClient.

@ericchiang
Copy link
Contributor Author

@jehiah any chance someone at bitly has bandwidth to chime in on this PR?

@mattenklicker
Copy link

@ericchiang I tried to use the oidc provider with dex, but without success.
After successful login in dex, oauth2-proxy tries to redeem the token directly from dex and this fails (it gets an error from dex):

2017/05/18 10:50:19 oauthproxy.go:527: 10.244.13.69:45572 ("xxxxx") error redeeming code token exchange: oauth2: cannot fetch token: 400 Bad Request
Response: {"error":"invalid_request","error_description":"redirect_uri did not match URI from initial request."}

I patched dex to see what is in redirect_uri (btw: debug logging isn't working in dex) and it's empty.
After looking with tcpdump, it seems it's unset:

...9RJ.*POST /token HTTP/1.1
Host: xxxxxx
X-Real-IP: xxxx
Connection: close
X-Forwarded-For: xxxx, xxxxx
X-Forwarded-Host: xxxx
X-Forwarded-Port: 443
X-Forwarded-Proto: https
X-Original-URI: /token
X-Scheme: https
Content-Length: 61
X-Amzn-Trace-Id: Root=xxxxxx
content-type: application/x-www-form-urlencoded
authorization: Basic xxxxxx
accept-encoding: gzip
user-agent: Go-http-client/2.0

code=xxxxxx&grant_type=authorization_code

Do you have any idea why?

BTW: -redeem-url seems to be ingnored

@ericchiang
Copy link
Contributor Author

@mattenklicker --redeem-url is implied from discovery metadata. There aren't spec compliant reasons to override it.

The error your seeing implies that the URL that the bitly proxy is sending as a redirect URI isn't registered with dex. This is the same issue you'd have with google, github, etc. if you didn't register the correct redirect. What flags are you running with and what does your staticClients dex config look like?

@bison
Copy link

bison commented May 22, 2017

I don't actually see the redirect URL being set in the oauth2.Config inside the Redeem() method. Am I just missing something?

@ericchiang
Copy link
Contributor Author

@bison the oauth2.Config is just used for token exchange. The Redeem URL reuses the proxy's built in OAuth2 redirects and is set here: https://github.com/bitly/oauth2_proxy/pull/389/files#diff-4aae454123f50b906f9aaef44f10011eR154

@bison
Copy link

bison commented May 22, 2017

@ericchiang right, but the the redirect_uri is sent to the provider during the exchange. It needs to be set there.

@mattenklicker
Copy link

@ericchiang the auth chain is: app -> oauth2-proxy -> dex -> ldap

app is a kubernetes ingress config
app ingress:

metadata:
  annotations:
    ingress.kubernetes.io/auth-signin: https://$host/oauth2/sign_in
    ingress.kubernetes.io/auth-url: https://$host/oauth2/auth
spec:
  rules:
  - host: kubernetes-dashboard....
    http:
      paths:
      - backend:
          serviceName: kubernetes-dashboard
          servicePort: 80
        path: /

and auth redirect ingress:

spec:
  rules:
  - host: kubernetes-dashboard....
    http:
      paths:
      - backend:
          serviceName: oauth2-proxy
          servicePort: 80
        path: /oauth2

oauth2-proxy config:

       args:
        - --provider=oidc
        - --redirect-url=https://oauth2-proxy..../oauth2/callback
        - --email-domain=*
        - --upstream=file:///dev/null
        - --http-address=0.0.0.0:4180
        - --cookie-secure=false
        - --oidc-issuer-url=https://dex....
        env:
        - name: OAUTH2_PROXY_CLIENT_ID
          value: oauth2_proxy
        - name: OAUTH2_PROXY_CLIENT_SECRET
          value: proxy
        - name: OAUTH2_PROXY_COOKIE_SECRET
          value: secret

dex static clients config:

staticClients:
- id: oauth2_proxy
  secret: proxy
  name: 'Oauth2-Proxy'
  redirectURIs:
  - 'https://oauth2-proxy..../oauth2/callback'

Note: The app-callback-URL itself is nowhere registered. But I don't think that's the problem here. If it should be registered, than it should be with oauth2-proxy (?). I tried to add it to redirectURIs without success.

Another question: --skip-provider-button let me guess, that this would skip the oauth2-proxy login page and would redirect directly to dex's login page. But it has no effect.

@benley
Copy link

benley commented Jun 2, 2017

I'm stuck on the same invalid_request error that @mattenklicker and others have mentioned. Does anyone know of a way to get this working?

@bison
Copy link

bison commented Jun 2, 2017

I still think that's because the redirect_uri is not being sent when requesting an access token. It needs to be set on the oauth2.Config. Otherwise this happens.

@benley
Copy link

benley commented Jun 2, 2017

Yep, sure enough. This is a one-line fix, and now it works:

diff --git a/providers/oidc.go b/providers/oidc.go
index 580c1e3..98d9220 100644
--- a/providers/oidc.go
+++ b/providers/oidc.go
@@ -23,6 +23,7 @@ func NewOIDCProvider(p *ProviderData) *OIDCProvider {
 func (p *OIDCProvider) Redeem(redirectURL, code string) (s *SessionState, err error) {
        ctx := context.Background()
        c := oauth2.Config{
+               RedirectURL:  redirectURL,
                ClientID:     p.ClientID,
                ClientSecret: p.ClientSecret,
                Endpoint: oauth2.Endpoint{

benley added a commit to postmates/oauth2_proxy that referenced this pull request Jun 2, 2017
@mattenklicker
Copy link

Nice. Works fine.
For documentation: You can omit --redirect-url=...., so you can use one proxy for all your apps.

@emmanuel
Copy link

As an OIDC (and OAuth) n00b, I'm a bit disoriented about where this PR stands. Can someone help me with the below questions?

  1. Is the PR held up from being merged by the aforementioned one line change?
  2. Does it work without that change so long as --redirect-url is omitted?
  3. What about the additional changes over in https://github.com/postmates/oauth2_proxy/commits/oidc-provider? (packaging, but also passing raw token in authorization header... important?)

@eicnix
Copy link

eicnix commented Jun 22, 2017

I tried this PR with the Keycloak as the OpenIDC Provider and it works fine but when I enable --pass-access-token=true the resulting cookie will be around 6mb and thus breaking the cookie size limit.

@benley
Copy link

benley commented Jun 23, 2017

@emmanuel I added the Authorization header because it makes oauth2_proxy work properly with the Kubernetes dashboard app, which uses the bearer token to act on the behalf of the authenticated user. It's probably not needed for most other apps.

@emmanuel
Copy link

@benley thanks for the explanation. that sounds like a good use-case (i.e., I'm off to deploy the proxy in front of kube-dashboard 😁 ).

Endpoint: oauth2.Endpoint{
TokenURL: p.RedeemURL.String(),
},
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this config need the RedirectURL? I was getting a 401 without it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I encountered the same as @travisofthenorth.
@ericchiang ?

@ericchiang
Copy link
Contributor Author

Comments addressed. Please let me know if there's anything else blocking this PR.

See the README for usage with Dex or any other OIDC provider.

To test run a backend:

    python3 -m http.server

Run dex and modify the example config with the proxy callback:

    go get github.com/coreos/dex/cmd/dex
    cd $GOPATH/src/github.com/coreos/dex
    sed -i.bak \
      's|http://127.0.0.1:5555/callback|http://127.0.0.1:5555/oauth2/callback|g' \
       examples/config-dev.yaml
    make
    ./bin/dex serve examples/config-dev.yaml

Then run the oauth2_proxy

    oauth2_proxy \
      --oidc-issuer-url http://127.0.0.1:5556/dex \
      --upstream http://localhost:8000 \
      --client-id example-app \
      --client-secret ZXhhbXBsZS1hcHAtc2VjcmV0 \
      --cookie-secret foo \
      --email-domain '*' \
      --http-address http://127.0.0.1:5555 \
      --redirect-url http://127.0.0.1:5555/oauth2/callback \
      --cookie-secure=false

Login with the username/password "admin@example.com:password"
@travisofthenorth
Copy link
Contributor

My feedback has been addressed 👍

@philips
Copy link
Contributor

philips commented Sep 8, 2017

@travisofthenorth great! Who can click merge?

Copy link
Member

@jehiah jehiah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 changes look good

@philips
Copy link
Contributor

philips commented Sep 12, 2017

🤣

@kfox1111
Copy link

I've almost got this working in front of kube-dashboard. I get authenticated through the proxy but the handoff of the token doesn't seem to make it to kube-dashboard... I see in the kube-dashboard 1.7 docs it mentions bearer tokens. I don't really see how they are provided in this ps though. Am I missing a flag? Any other ideas?

@benley
Copy link

benley commented Sep 26, 2017

Are you using a build that's patched to pass the user's raw token along in a Authorization header? Here's what finally got that part working for me: postmates@5a27234

@kfox1111
Copy link

I checked out trunk from earlier today. Is there still patches needing to be merged?

@kfox1111
Copy link

yeah.... looks like that other patch is not here yet... Thanks for the pointer to it. Any plans for a pr here?

@benley
Copy link

benley commented Sep 26, 2017

I hadn't opened a PR for that feature because I have been assuming that it's not a feature that would be accepted upstream, since it's rather specific to one auth backend and a use case that is nonstandard. At least I think it's nonstandard...?

@kfox1111
Copy link

I could see other things wanting to use bearer tokens too? Seems like a very minor thing to fork a project over. If upstream will take it, probably worth a shot?

@kfox1111
Copy link

tried to cherry pick the one patch. it fails due to:
./oauthproxy.go:690:61: session.RawIDToken undefined (type *providers.SessionState has no field or method RawIDToken)

Looks like the two code bases are farther apart then I had hoped. :(

@kfox1111
Copy link

I think I figured it out. 'go get' was fetching a new checkout of the bitly/oauth2_proxy rather then reusing the existing, patched checkout. After patching the downloaded source, the patch seems to have worked as is.

@jehiah jehiah added this to the v2.3 milestone Oct 23, 2017
@a-chernykh
Copy link

Any chance we can get this patch in the upstream?

@emmanuel
Copy link

I would also be interested in this patch being incorporated in the official project, for the same purpose[1]. Basically, "would it hurt to include it upstream"?

[1] for use with the Kubernetes dashboard, which requires the actual auth JWT to enable the dashboard to impersonate/operate-on-behalf-of the logged in user

@Drupi
Copy link

Drupi commented Aug 3, 2018

Hello ! Can some1 share me a container with this PR merged

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.