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.

Sign up for GitHub

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

Jump to bottom

ADFS support / top level config elements are only vouch and oauth #65

Closed
simongottschlag opened this issue Feb 5, 2019 · 18 comments
Closed

ADFS support / top level config elements are only vouch and oauth #65

simongottschlag opened this issue Feb 5, 2019 · 18 comments

Comments

@simongottschlag
Copy link
Contributor

Hi,

Sorry if this is me not understanding Go. I've been looking at an issue where my cookie.domain isn't being set for the cookies.

I've forked and built the project and added the following:
log.Debugf("temp debug - cookie: %v", cfg.Cfg.Cookie)

Which is returned like this:
time="2019-02-05T10:17:18Z" level=debug msg="temp debug - cookie: {VouchCookie false true}"

I can see, at startup, that it is configured. Seems like cookie.go isn't using the configuration set?

Am I just doing everything wrong here or is there a bug?
@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019 via email

@simongottschlag
Copy link
Contributor Author

simongottschlag commented Feb 5, 2019 •
edited
Loading

This is the output:

time="2019-02-05T10:58:53Z" level=debug msg="map[vouch:map[allowallusers:true loglevel:debug port:80 listen:0.0.0.0] oauth:map[scopes:[openid email profile] user_info_url:https://adfs.example.com/adfs/userinfo callback_url:https://auth.example.com/auth provider:oidc client_id:clientId token_url:https://adfs.example.com/adfs/oauth2/token/ auth_url:https://adfs.example.com/adfs/oauth2/authorize/ client_secret:***] cookie:map[httponly:false secure:false name:VouchCookieTest domain:example.com] headers:map[jwt:X-Vouch-Token querystring:access_token redirect:X-Vouch-Requested-URI] session:map[name:VouchSession]]"

When reading it from main it looks like this: (used %s instead of %v)

time="2019-02-05T10:58:53Z" level=debug msg="debug - main cfg {debug 0.0.0.0 %!s(int=80) [] [] %!s(bool=true) %!s(bool=false) {%!s(int=240) Vouch *** %!s(bool=true)} {VouchCookie  %!s(bool=false) %!s(bool=true)} {X-Vouch-Token X-Vouch-User access_token X-Vouch-Requested-URI X-Vouch-Success} {data/vouch_bolt.db} {VouchSession ***}  [] %!s(bool=false) %!s(bool=false)}"

It really seems like I'm not getting the settings to other places other than cfg.go.

This is the config.yml:

vouch:
  logLevel: debug
  listen: 0.0.0.0
  port: 80
  AllowAllUsers: true
cookie: 
  name: VouchCookieTest
  domain: example.com
  secure: false
  httpOnly: false
headers:
  jwt: X-Vouch-Token
  querystring: access_token
  redirect: X-Vouch-Requested-URI
session:
  name: VouchSession
oauth:
  provider: oidc
  client_id: clientId
  client_secret: ***
  auth_url: https://adfs.example.com/adfs/oauth2/authorize/
  token_url: https://adfs.example.com/adfs/oauth2/token/
  user_info_url: https://adfs.example.com/adfs/userinfo
  scopes:
    - openid
    - email
    - profile
  callback_url: https://auth.example.com/auth

This is the nginx-config:

user  nginx;
worker_processes  3;

error_log  /var/log/nginx/error.log warn;
pid        /var/run/nginx.pid;


events {
    worker_connections  1024;
}


http {
    include       /etc/nginx/mime.types;
    default_type  application/octet-stream;

    log_format  main  '$remote_addr - $remote_user [$time_local] "$request" '
                      '$status $body_bytes_sent "$http_referer" '
                      '"$http_user_agent" "$http_x_forwarded_for"';

    access_log  /var/log/nginx/access.log  main;
    sendfile        on;
    keepalive_timeout  65;

    server {
        listen       80;
        server_name  dev.example.com;

        # send all requests to the `/validate` endpoint for authorization
        auth_request /validate;

        location = /validate {
          # Vouch Proxy can run behind the same nginx-revproxy
          # May need to add "internal", and comply to "upstream" server naming
          proxy_set_header Host dev.example.com;
          proxy_pass http://vouch-proxy;

          # Vouch Proxy only acts on the request headers
          proxy_pass_request_body off;
          proxy_set_header Content-Length "";

          # pass X-Vouch-User along with the request
          auth_request_set $auth_resp_x_vouch_user $upstream_http_x_vouch_user;

          # these return values are used by the @error401 call
          auth_request_set $auth_resp_jwt $upstream_http_x_vouch_jwt;
          auth_request_set $auth_resp_err $upstream_http_x_vouch_err;
          auth_request_set $auth_resp_failcount $upstream_http_x_vouch_failcount;
        }

        # if validate returns `401 not authorized` then forward the request to the error401block
        error_page 401 = @error401;

        location @error401 {
            # redirect to Vouch Proxy for login
            return 302 https://auth.example.com/login?url=$scheme://$http_host$request_uri&vouch-failcount=$auth_resp_failcount&X-Vouch-Token=$auth_resp_jwt&error=$auth_resp_err;
        }
        
        location / {
            proxy_pass http://dev.default.svc.cluster.local/;
            auth_request_set $auth_resp_x_vouch_user $upstream_http_x_vouch_user;
        }
    }

    server {
        listen 80;
        server_name auth.example.com;
        location / {
          proxy_set_header Host auth.example.com;
          proxy_pass http://vouch-proxy;
        }
    }

    server {
        listen       8088;
        server_name  _;
        
        location /healthz {
            stub_status;
            access_log off;
            allow all;
        }
    }
}

@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019 via email

@simongottschlag
Copy link
Contributor Author

Hi,

I updated the above with the nginx-configuration. Is something missing form it?

@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019

My apologies, the edits don't trigger a notification via email so I assumed you had not posted the config.

You're using allowAllUsers without a domain. Could you try setting that to false and setting 'vouch.domains: - example.com' and see if that works.

@simongottschlag
Copy link
Contributor Author

My bad! The issue with ADFS is that we're not receiving an email from the user info enpdoint, only getting a "sub". So I can't do that and still be able to login.

@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019

That seems to be a common problem with oidc providers. Everyone follows the spec but the userinfo json blob is always a bit different.

Vouch Proxy will not be able to construct a jwt if it's not able to parse the userinfo properly.

We haven't had any questions regarding ADFS support. I can't find any mention of the json ADFS returns. Could you post that from your logs or provide a link to a spec. Are you running this in Azure? Happy to add the support for ADFS if you don't mind testing it for me over the next day or so.

@simongottschlag
Copy link
Contributor Author

Hi,

Sure! No problem to help with this. But I'm still not understanding why we're not seeing the correct configuration in cfg.Cfg when called from (for example) main.go or cookie.go. I am able to print out viper.AllSettings() but then I'm not seeing the defaults.

The userinfo endpoint prints it like this:

time="2019-02-05T11:51:51Z" level=info msg="OpenID userinfo body:  {\"sub\":\"gNrph/aMvDk6PPPt+EnqX+oXsIvosWQ0qJhesBj70vk=\"}"

The best thing would be if we could extract the id_token using implicit flow, or through the token endpoint.

You will also have to change this to get it working (this was the first issue I hit): https://github.com/simongottschlag/vouch-proxy/blob/adfs_tests/handlers/handlers.go#L51 (Changing to URLEncoding)

@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019 via email

@simongottschlag
Copy link
Contributor Author

Ok! Great!

Trying my best at troubleshooting but really hard when I have no clue about the syntax

@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019 via email

@bnfinet
Copy link
Member

bnfinet commented Feb 5, 2019

Looks like lots of folks are struggling to get the userinfo endpoint to return more user info 😁

https://social.technet.microsoft.com/Forums/Lync/en-US/679a7998-3836-4526-9685-9386744f031c/the-access-token-in-the-request-doesnt-have-required-audience?forum=ADFS

@simongottschlag
Copy link
Contributor Author

Yep! Lots of issues with it unfortunately.

I've already tried combinations of keycloak-gatekeeper, envry/oidc-proxy (nginx+lua) and DEX. Gotten far but the last issue was keycloak-gatekeeper intercepting the Authorization-header which meant I'm not able to send basic auth to a backend after the authentication.

The issue I'm seeing right now (after the URLEncoding fix) is that I'm not getting the cookies to domain.com and only auth.domain.com wich causes a loop.

I'm not sure if I'm doing something wrong here or is there a problem with the cfg.Cfg in other modules? I'm only getting the "default" settings when printing it in main and cookie.

@simongottschlag
Copy link
Contributor Author

Hi,

Was able to fix the issue (at least for myself) with Cookie.Domain: #66

Here you have for the state to support ADFS as well: #67

Right now, I'm getting the following:

time="2019-02-05T15:38:34Z" level=debug msg=/validate
time="2019-02-05T15:38:34Z" level=debug msg=cookie cookieName=VouchCookie cookieValue="***"
time="2019-02-05T15:38:34Z" level=debug msg="jwt from cookie: ***"
time="2019-02-05T15:38:34Z" level=debug msg="tokenString ***"
time="2019-02-05T15:38:34Z" level=debug msg="decompressed tokenString ***"
time="2019-02-05T15:38:34Z" level=debug msg="*ptokenCLaims: { [] { 1549395514  0 Vouch 0 }}"
time="2019-02-05T15:38:34Z" level=error msg="no Username found in jwt"

Which means we need to solve the issue with ADFS. I'll think about what the best way could be, but I know implicit works fine. (using it for Kubernetes)

@simongottschlag
Copy link
Contributor Author

Hi,

I've created a new PR wich seems to be working with ADFS: #68

You should most likely take a look at the code, since I've just made a mess. :)

@bnfinet
Copy link
Member

bnfinet commented Feb 8, 2019

@simongottschlag unfortunately I'm not able to reproduce the behavior that you are seeing with setting the domain for the cookie.

With master/v0.4.9, when I set vouch.cookie.domain it results in a cookie being offered which is in the domain I have specified. I have tested this under a variety of scenarios on two servers.

@bnfinet
Copy link
Member

bnfinet commented Feb 8, 2019 •
edited
Loading

I notice now that the config that you have offered has...

vouch:
  logLevel: debug
  listen: 0.0.0.0
  port: 80
  AllowAllUsers: true
cookie: 
  name: VouchCookieTest
  domain: example.com
  secure: false
  httpOnly: false
headers:
  jwt: X-Vouch-Token
  querystring: access_token
  redirect: X-Vouch-Requested-URI
session:
  name: VouchSession

Which places vouch and cookie at the same indentation in the yaml (so cookie.domain, not vouch.cookie.domain). This is a configuration error.

bnfinet added a commit that referenced this issue Feb 8, 2019
@bnfinet
#65 check your indentation (yaml is so spacey)
e09ad22
@simongottschlag simongottschlag mentioned this issue Feb 8, 2019
Closed
@simongottschlag
Copy link
Contributor Author

@bnfinet yeah, I'm so sorry! This is my bad. Adjusted it and now is working.
I've removed the "cookie fix" and created on PR for all the changes that I'm using right now: #72

Is it OK that we close the other two and just work based on this one? Seems like I'm messing up GIT all the time :)

@bnfinet bnfinet changed the title Cookie Domain not being imported ADFS support / top level config elements are only vouch and oauth Feb 18, 2019
@bnfinet bnfinet mentioned this issue Feb 18, 2019
Closed
bnfinet added a commit that referenced this issue May 22, 2020
@bnfinet
#65 check your indentation (yaml is so spacey)
eef9c6f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bnfinet @simongottschlag