arrikto / oidc-authservice

This is a fork/refactoring of the ajmyyra/ambassador-auth-oidc project
MIT License
87 stars 65 forks source link

Using with Ambassador AuthService proxying to several hosts #43

Open asetty opened 3 years ago

asetty commented 3 years ago

Hi there I've been testing using the auth service with Ambassador AuthService. I'm stuck right now and am not sure if I have a configuration issue with Ambassador/Envoy or I was misunderstanding the capabilities of oidc-authservice. Specifically, in the OIDC callback function the request is redirected to the auth service host at oidc-auth.company.io/ORIGINAL_PATH instead of serviceA.company.io/ORIGINAL_PATH which is the host from the original request before being directed to the auth service and going through the OIDC flow. I've added some logging to the code and see that the request.URL.String() that is used for the original path in the state only includes path, without host (code: https://github.com/arrikto/oidc-authservice/blob/master/state.go).

So right now I'm stucking wondering if:

  1. I have some Ambassador/Envoy config issue and the host is being stripped in the request URL
  2. oidc-authservice is meant to be used to authenticate/authorize endpoints on a single service behind the edge proxy rather than multiple (which would be a little weird since AuthService object in Ambassador applies to all Mappings)
  3. something else?

Let me know if you have any insights or need more info. If 2 is the case I can look into forking this and/or starting a project to suit my needs (also would like a token based auth option rather than session based).

A bit more details about the setup:

Ambassador API Gateway (open source) is being used as the edge proxy for a k8s cluster. I have several services that are accessible through the proxy:

serviceA.company.io
serviceB.company.io
serviceC.company.io
...

An Ambassador Mapping for each service e.g.:

apiVersion: ambassador/v1
kind:  Mapping
name:  serviceA_mapping
ambassador_id: edge_proxy
host: serviceA.company.io
prefix: /
service: serviceA.namespaceA:9000

The authservice is deployed and configured as so:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: oidc-auth
  namespace: edge-proxy
  labels:
    app: oidc-auth
spec:
  selector:
    matchLabels:
      app: oidc-auth
  replicas: 1
  template:
    metadata:
      labels:
        app: oidc-auth
    spec:
      serviceAccountName: edge-proxy
      containers:
      - name: oidc-auth
        image: arrikto/oidc-authservice:auth_test
        ports:
        - name: http-api
          containerPort: 8080
        - name: http-web
          containerPort: 8082
        - name: http-readiness
          containerPort: 8081
        readinessProbe:
          httpGet:
            path: /
            port: 8081
        - name:  OIDC_PROVIDER
          value: https://accounts.google.com
        - name: OIDC_SCOPES
          value: "openid profile email"
        - name: AUTHSERVICE_URL_PREFIX
          value: "https://oidc-auth.company.io/authservice/"
        - name: SKIP_AUTH_URLS
          value: ""
        - name: CLIENT_ID
          valueFrom:
            secretKeyRef:
              name: oidc-auth-secret
              key: CLIENT_ID
        - name: CLIENT_SECRET
          valueFrom:
            secretKeyRef:
              name: oidc-auth-secret
              key: CLIENT_SECRET
        volumeMounts:
        - mountPath: "/var/lib/authservice"
          name: data
      volumes:
      - name: google-oauth
        secret:
          secretName: oidc-auth-secret
      - name: data
        emptyDir: {}
apiVersion: v1
kind: Service
metadata:
  name: oidc-auth
  namespace: edge-proxy
  annotations:
    getambassador.io/config: |
      ---
      apiVersion: ambassador/v2
      kind:  AuthService
      name:  authentication
      ambassador_id: edge_proxy
      auth_service: "oidc-auth.edge-proxy.svc.cluster.local:8080"
      allowed_request_headers:
      - "X-Auth-Userinfo"
      - "X-Auth-Token"
      - "authorization"
      - "cookie"
      - "Authorization"
      - "Referer"
      allowed_authorization_headers:
      - "X-Auth-Userinfo"
      - "authorization"
      - "cookie"
      - "Authorization"
      - "X-Auth-Token"
      - "Referer"
      ---
      apiVersion: ambassador/v2
      kind:  Mapping
      name:  ambassador_oidc_auth_mapping
      ambassador_id: edge_proxy
      prefix: "/authservice"
      service: "oidc-auth.edge-proxy:8080"
      host: oidc-auth.company.io
spec:
  type: ClusterIP
  selector:
    app: oidc-auth
    app.kubernetes.io/name: oidc-auth
  ports:
  - port: 8080
    name: http-oidc-auth
    targetPort: http-api
yanniszark commented 3 years ago

@asetty first of all, thanks a lot for the detailed issue! We mainly use the AuthService with Istio (as you can also see from the CI tests), so I have limited experience with Ambassador. More specifically, we mainly use path-based routing (instead of host-based) and I guess that's the reason we hadn't bumped into something like this yet.

What you are doing is different from how we are using the AuthService so far. You are using a separate subdomain for the AuthService. It's actually meant to live under a path in the app's subdomain. Otherwise, things like setting the cookie after authentication won't work (subdomains must be explicitly specified in set-cookie and they aren't right now).

Also, since the HTTP authorization request comes from the Gateway (2nd arrow, 1st line: https://github.com/arrikto/oidc-authservice/blob/master/docs/media/oidc_authservice_sequence_diagram.svg), I don't know if the information about the original host requested by the user is even present in the authorization request.

asetty commented 3 years ago

@yanniszark thanks for your answer. Things make a lot of sense now that I know for sure this was meant for hosting the auth service under a path in a single apps subdomain. In my case it makes this tool pretty weird to use with Ambassador's AuthService plugin because when you define an AuthService it applies to all Mappings defined for the Ambassador instance (unless the Mapping specifies bypass_auth). So I could probably make it work for a single service, and this service could even be yet another proxy to the actual services, but i'd like to avoid that extra layer lol. Doc: https://www.getambassador.io/docs/latest/topics/running/services/auth-service/ In any case, I'm thankful for this project because it will be a good starting point and has helped me gain some insight into approaching my problem.

I have adding some debugging prints to the code and I can see that the original host is propagated from the gateway (ambassador in my case) to the auth service and can be found at r.Host. I think I could pretty easily make some changes to your project integrate this with the session_authenticator to also save the host in the state. Or maybe there is a way that we can send the full url host + path to the OIDC provider (as a query param, body, or header...) so that it will be included with the request from OIDC Provider to the auth service's callback handler?

I'll need to gain some understanding of the details, but will probably need to handle what you mentioned:

Otherwise, things like setting the cookie after authentication won't work (subdomains must be explicitly specified in set-cookie and they aren't right now).

Thanks for pointing that out.

Let me know if you have any other info or pointers and I'll try to hack around the next couple days to get something working for my use case.

yanniszark commented 3 years ago

I have adding some debugging prints to the code and I can see that the original host is propagated from the gateway (ambassador in my case) to the auth service and can be found at r.Host.

I also confirm that I can see the original host when using Istio (Envoy) with the ext_authz filter. This is a great thing to know for future enhancements. Let me know if you ended up somewhere on the design you want to follow.

bmariesan commented 3 years ago

@asetty Have got any progress on this issue? We're also trying to do an Ambassador integration and we seem to be having a rather similar issue

asetty commented 3 years ago

@bmariesan Yeah I have something working for my use case now. Going to clean up the changes and try to send some pull requests soon, but also have some other changes I want to make first (e.g. support group claims for google) so we'll see what I get to first.

If you're interested I have a branch here with some work: https://github.com/asetty/oidc-authservice/tree/hosts The scheme for the original request URL is hardcoded as https right now, and you'll see a few other TODOs. To use it you can set the new SESSION_DOMAIN env var I created and set it to the shared domain of your ambassador services.

Right now now I'm also relying on SESSION_DOMAIN to choose between using the relative URL for origURL in newState, or propagating the scheme and host. I'm not sure if this is the right design choice. Other options could be:

  1. the user chooses this with a new env var, maybe REQUEST_STRATEGY, or some other better name
  2. we always use schemeAndHost
  3. something else

Let me know if you have any feedback or this helps any.

asetty commented 3 years ago

This pull request is a first step in making the necessary upstream changes: https://github.com/arrikto/oidc-authservice/pull/48

asetty commented 3 years ago

Adding some details to this issue @yanniszark

Current state:

The AuthService is hosted on a reversed path in the app/resource server's subdomain. When saving the original request state, only the URL / path is saved http.Redirect back to the original URL in the callback function will be interpreted as relative to the host used by the OIDC Provider to contact the auth service.

Problem:

In many setups this will work, but if using host-based routing with an edge proxy the current implementation does not nicely cover the use case (for an example of this sort of setup see the first post in the issue). No matter what host the original request was for, it will be redirected to the domain hosting the AuthService which will not have the original resource. It may be possible for the authservice to be "hosted" on the "/authservice" path of EVERY app's subdomain, but I'm not sure it is possible to specify multiple redirectURIs for the OIDCProvider callback. Even then we would still need to propagate some information about the original host to the OIDCProvider.

Proposed Solution:

Enable auth service to be hosted on it's own subdomain and provide auth for N hosts/services under a shared subdomain (e.g. serves all hosts *.company.io)

In order to do this there will need to be 2 changes:

  1. Save original request's host and scheme/proto in the newState function
  2. Set the Domain attribute on Set-Cookie to the shared domain.

Functionally with these changes, authentication will be for the entire subdomain / AuthService, but authorization with be per request. Meaning if a user is authenticated to serviceA.company.io the same session will be used for serviceB.company.io, however authorization will still be checked for each request. Maybe there's a case to do session per host, but for my use case it is not needed, but could be supported later if wanted.

I have a draft PR I posted to the aristanetworks fork here https://github.com/aristanetworks/oidc-authservice/pull/1 that implements the described functionality. The env vars introduced are:

  1. SESSION_DOMAIN - domain attribute for session cookie (default: "")
  2. MULTIPLE_HOSTS - enable saving scheme and host vs. relativeURL (default: false)
  3. SCHEME_DEFAULT - original request scheme to use if not provided in header (default: https)
  4. SCHEME_HEADER - header to find scheme for original request (default should probably be X-Forwarded-Proto)

I tried to preserve the original functionality completely with my PR, but I have been going back and forth on the explicit MULTIPLE_HOSTS env var. Copying from the draft PR -- the lingering questions I've been having around the env vars in general.

Topics for discussion:

1. Rather than MULTIPLE_HOSTS we could overload the SESSION_DOMAIN env var to also control if we use relative URL vs propagate scheme and host. Could rename to AUTH_DOMAIN or something else in this case since it is used for more than just Domain attribute of cookie.
2. Maybe we can get rid of relativeURL and always propagate scheme and host?
3. Should we validate that the Host is a subdomain of SESSION_DOMAIN?
4. Maybe rename SCHEME_* env vars to something else. e.g. REQ_PROTO_*

Thanks and let me know if you need anymore details :) cc @tsuna

asetty commented 3 years ago

@yanniszark Updated https://github.com/aristanetworks/oidc-authservice/pull/1 a bit since posting my last comment. Namely, I got rid of the MULTIPLE_HOSTS env var and decided to rely on SESSION_DOMAIN instead which seems to make sense. Would appreciate your feedback and then I can either update my existing PR or create a new one with both commits.

asetty commented 3 years ago

Hi @yanniszark. I've been running oidc-authservice with my changes to support a host + path based routing setup as I've described without issues for the past month and a half.

I've just rebased my changes on top of the ones you made changing the handling of the OIDC state parameter and would like to send a PR soon. Would like to get your feedback on the details above, most of which haven't changed.

yanniszark commented 3 years ago

@asetty thanks for pinging on this. So, if I understand correctly, the following changes are needed:

asetty commented 3 years ago

Yep this is correct. Getting rid of the setting for scheme header and just always using X-Forwarded-Proto sounds good to me since it's pretty standard.

One question I have regarding the implementation is, do you think we should change the code to ALWAYS save scheme and host for the original request, or maybe depending on SESSION_DOMAIN env var being set the behavior could be different. It's definitely simpler to only have one strategy for saving the original request state, but using the relative URL and not setting the session domain as in the current implementation may be nice in setups where all routing is path based.

Let me know what you think and then I'll spruce up my changes a bit and send a PR!

asetty commented 3 years ago

Another question. What do you think we should do in the case where there is no X-Forwarded-Proto header present? One idea is to have a default value to fall back on, maybe configured by a new env var e.g. DEFAULT_PROTOCOL, SCHEME_DEFAULT, REQ_PROTO_DEFAULT with a default of "https"... Seems better to have a default than throw an error at least.

asetty commented 3 years ago

@yanniszark been getting unlucky with illnesses the past couple weeks so have not been able to get the PR pushed. I'll try to do it in the next couple days before the today, it's mostly ready though. LMK if you have any feedback about the above, otherwise we can address it on the PR. Thanks!

matej-topolovec commented 1 year ago

Hi @asetty

Having the similiar issue as you described. Did you manage to solve it?

Ambassador AuthService config

apiVersion: getambassador.io/v2
kind: AuthService
metadata:
  name: authentication
  namespace: emissary-ingress
spec:
  add_linkerd_headers: false
  allowed_authorization_headers:
  - X-Auth-Userinfo
  - authorization
  - cookie
  - Authorization
  - X-Auth-Token
  - Referer
  allowed_request_headers:
  - X-Auth-Userinfo
  - X-Auth-Token
  - authorization
  - cookie
  - Authorization
  - Referer
  ambassador_id:
  - --apiVersion-v3alpha1-only--default
  auth_service: oidc-authservice.apps:8080
  path_prefix: /authservice

oidc-authservice k8s service

apiVersion: v1
kind: Service
metadata:
  labels:
    app.kubernetes.io/instance: oidc-authservice
    app.kubernetes.io/name: oidc-authservice
  name: oidc-authservice
  namespace: apps
spec:
  clusterIP: 100.69.240.123
  clusterIPs:
  - 100.69.240.123
  internalTrafficPolicy: Cluster
  ipFamilies:
  - IPv4
  ipFamilyPolicy: SingleStack
  ports:
  - name: http-authservice
    port: 8080
    protocol: TCP
    targetPort: http-api
  - name: http-web
    port: 8082
    protocol: TCP
    targetPort: http-web
  selector:
    app.kubernetes.io/instance: oidc-authservice
    app.kubernetes.io/name: oidc-authservice
  sessionAffinity: None
  type: ClusterIP

oidc-authservice ambassador mapping

apiVersion: getambassador.io/v2
kind: Mapping
metadata:
  labels:
    app.kubernetes.io/instance: oidc-authservice
    app.kubernetes.io/managed-by: Helm
  name: oidc-authservice
  namespace: apps
spec:
  ambassador_id:
  - --apiVersion-v3alpha1-only--default
  bypass_auth: true
  host: login.company.com
  prefix: /authservice/
  service: oidc-authservice.apps:8080

So the issue is when I go to the example.company.com I get the logs:

_time="2023-03-14T13:14:47Z" level=info msg="URI is whitelisted. Accepted without authorization." host=example.company.com ip="" method=GET path="/authservice/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff2?v=4.7.0"_

I've set the SESSION_DOMAIN variable to company.com and I've built the docker image on top of this project, and on top of https://github.com/aristanetworks/oidc-authservice (master and arista-dev branch, as your pull request is merged in arista-dev branch) and got the same result.

As if *.company.com are somehow whitelisted and it only works if I go to https://login.company.com/authservice/site/homepage . than it redirects me to OIDC provider but again I get ERR_TOO_MANY_REDIRECTS, once I login.

Thanks!

edwardzjl commented 1 year ago

I'm also interested in this feature.

We are not using Ambassador, istio only. But we want to deploy multiple apps along with kubeflow in the same kubernetes cluster. As far as I know it's difficult to deploy kubeflow under a sub path, as I commented here https://github.com/arrikto/oidc-authservice/issues/97#issuecomment-1538110150 (correct me if I'm wrong). The author of that issue might have the same requirement, by trying to deploy kubeflow under some sub-path.

Current state is that oidc-authservice, deployed with kubeflow, will authenticate all requests to "/", and redirect to the host name that oidc-authservice was deployed under.

It would be great if oidc-authservice can authenticate multi-host requests, so that it can be achieved by deploying different apps to different host names, say:

With that said, I think we need to take more into consideration. For example the white list, i.e. SkipAuthURLs.

As for now oidc-authservice white list requests by their path only, which will become a problem when authenticating multiple hosts.

We have a simple implementation running inspired by @asetty 's PR, modified on the master branch (without the whitelist thing).

Appreciate if we can have some discussion and make some progress on this.