Skip to content

Conversation

@tahoward
Copy link
Contributor

@tahoward tahoward commented Nov 10, 2018

Problem

Addresses: #32

Solution

Add new parameters:
AUTH: SSO_EMAIL_ADDRESSES
PROXY: EMAIL_ADDRESSES

If parameters provided they override SSO_EMAIL_DOMAIN and EMAIL_DOMAIN respectively.
New parameters use a validater to check full e-mail address.

Notes

Example deployment:

---
apiVersion: v1
kind: Namespace
metadata:
  labels:
    k8s-app: sso
  name: sso
---
apiVersion: v1
kind: Secret
metadata:
  labels:
    k8s-app: sso
  name: sso-authenticated-emails
  namespace: sso
type: Opaque
data:
  emails: "<BASE64_SECRET>"
---
apiVersion: v1
kind: Secret
metadata:
  labels:
    k8s-app: sso
    component: auth
  name: sso-auth-secrets
  namespace: sso
type: Opaque
data:
  CLIENT_ID: "<BASE64_SECRET>"
  CLIENT_SECRET: "<BASE64_SECRET>"
  AUTH_CODE_SECRET: "<BASE64_SECRET>"
  COOKIE_SECRET: "<BASE64_SECRET>"
---
apiVersion: v1
kind: Secret
metadata:
  labels:
    k8s-app: sso
    component: proxy
  name: sso-proxy-secrets
  namespace: sso
type: Opaque
data:
  CLIENT_ID: "<BASE64_SECRET>"
  CLIENT_SECRET: "<BASE64_SECRET>"
  AUTH_CODE_SECRET: "<BASE64_SECRET>"
  COOKIE_SECRET: "<BASE64_SECRET>"
---
apiVersion: v1
kind: ConfigMap
metadata:
  labels:
    k8s-app: sso
  name: sso-upstream-configs
  namespace: sso
data:
  upstream_configs.yml: |-
    - service: hello-world
      default:
        from: hello-world.sso.domain.com
        to: http://hello-world.default.svc.cluster.local
---
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  labels:
    k8s-app: sso
    component: proxy
  name: sso-proxy
  namespace: sso
spec:
  replicas: 1
  template:
    metadata:
      labels:
        k8s-app: sso
        component: proxy
    spec:
      containers:
      - image: tahoward/sso
        name: sso-proxy
        command: ["/bin/sso-proxy"]
        ports:
        - containerPort: 4180
        env:
          - name: EMAIL_ADDRESSES
            valueFrom:
              secretKeyRef:
                name: sso-authenticated-emails
                key: emails
          - name: UPSTREAM_CONFIGS
            value: /sso/upstream_configs.yml
          - name: PROVIDER_URL
            value: https://auth.domain.com
          - name: CLIENT_ID
            valueFrom:
              secretKeyRef:
                name: sso-proxy-secrets
                key: CLIENT_ID
          - name: CLIENT_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-proxy-secrets
                key: CLIENT_SECRET
          - name: AUTH_CODE_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-proxy-secrets
                key: AUTH_CODE_SECRET
          - name: COOKIE_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-proxy-secrets
                key: COOKIE_SECRET
          # STATSD_HOST and STATSD_PORT must be defined or the app wont launch, they dont need to be a real host / port, but they do need to be defined.
          - name: STATSD_HOST
            value: localhost
          - name: STATSD_PORT
            value: "11111"
          - name: COOKIE_SECURE
            value: "false"
          - name: CLUSTER
            value: dev
          - name: VIRTUAL_HOST
            value: "*.sso.domain.com"
        readinessProbe:
          httpGet:
            path: /ping
            port: 4180
            scheme: HTTP
        livenessProbe:
          httpGet:
            path: /ping
            port: 4180
            scheme: HTTP
          initialDelaySeconds: 10
          timeoutSeconds: 1
        resources:
          limits:
            memory: "256Mi"
            cpu: "200m"
        volumeMounts:
        - name: upstream-configs
          mountPath: /sso
      volumes:
        - name: upstream-configs
          configMap:
            name: sso-upstream-configs
---
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  labels:
    k8s-app: sso
    component: auth
  name: sso-auth
  namespace: sso
spec:
  replicas: 1
  template:
    metadata:
      labels:
        k8s-app: sso
        component: auth
    spec:
      containers:
      - image: tahoward/sso
        name: sso-auth
        command: ["/bin/sso-auth"]
        ports:
        - containerPort: 4180
        env:
          - name: SSO_EMAIL_ADDRESSES
            valueFrom:
              secretKeyRef:
                name: sso-authenticated-emails
                key: emails
          - name: HOST
            value: auth.domain.com
          - name: REDIRECT_URL
            value: https://auth.domain.com
          - name: PROXY_ROOT_DOMAIN
            value: domain.com
          - name: CLIENT_ID
            valueFrom:
              secretKeyRef:
                name: sso-auth-secrets
                key: CLIENT_ID
          - name: CLIENT_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-auth-secrets
                key: CLIENT_SECRET
          - name: PROXY_CLIENT_ID
            valueFrom:
              secretKeyRef:
                name: sso-proxy-secrets
                key: CLIENT_ID
          - name: PROXY_CLIENT_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-proxy-secrets
                key: CLIENT_SECRET
          - name: AUTH_CODE_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-auth-secrets
                key: AUTH_CODE_SECRET
          - name: COOKIE_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-auth-secrets
                key: COOKIE_SECRET
          # OLD_COOKIE_SECRET is the same as COOKIE_SECRET, not sure why its even needed at this point
          - name: OLD_COOKIE_SECRET
            valueFrom:
              secretKeyRef:
                name: sso-auth-secrets
                key: COOKIE_SECRET
          # STATSD_HOST and STATSD_PORT must be defined or the app wont launch, they dont need to be a real host / port
          - name: STATSD_HOST
            value: localhost
          - name: STATSD_PORT
            value: "11111"
          - name: COOKIE_SECURE
            value: "false"
          - name: CLUSTER
            value: dev
          - name: VIRTUAL_HOST
            value: auth.domain.com
        readinessProbe:
          httpGet:
            path: /ping
            port: 4180
            scheme: HTTP
        livenessProbe:
          httpGet:
            path: /ping
            port: 4180
            scheme: HTTP
          initialDelaySeconds: 10
          timeoutSeconds: 1
        resources:
          limits:
            memory: "256Mi"
            cpu: "200m"
---
apiVersion: v1
kind: Service
metadata:
  labels:
    k8s-app: sso
    component: proxy
  name: sso-proxy
  namespace: sso
spec:
  ports:
  - port: 80
    targetPort: 4180
    name: http
  selector:
    k8s-app: sso
    component: proxy
---
apiVersion: v1
kind: Service
metadata:
  labels:
    k8s-app: sso
    component: auth
  name: sso-auth
  namespace: sso
spec:
  ports:
  - port: 80
    targetPort: 4180
    name: http
  selector:
    k8s-app: sso
    component: auth
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  labels:
    k8s-app: sso
  annotations:
    certmanager.k8s.io/cluster-issuer: letsencrypt
    certmanager.k8s.io/acme-challenge-type: dns01
    certmanager.k8s.io/acme-dns01-provider: cloudflare
  name: sso-ingress
  namespace: sso
spec:
  rules:
  - host: auth.domain.com
    http:
      paths:
      - backend:
          serviceName: sso-auth
          servicePort: 80
        path: /
  - host: "*.sso.domain.com"
    http:
      paths:
      - backend:
          serviceName: sso-proxy
          servicePort: 80
        path: /
  tls:
  - secretName: sso-auth-cert
    hosts: 
    - auth.domain.com
  - secretName: sso-proxy-cert
    hosts: 
    - "*.sso.domain.com"
---
# HELLO WORLD
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  name: hello-world
  labels:
    k8s-app: hello-world
spec:
  replicas: 1
  template:
    metadata:
      labels:
        k8s-app: hello-world
    spec:
      containers:
      - image: tutum/hello-world:latest
        name: hello-world
        ports:
        - containerPort: 80
---
kind: Service
apiVersion: v1
metadata:
  labels:
    k8s-app: hello-world
  name: hello-world
spec:
  ports:
  - port: 80
  selector:
    k8s-app: hello-world

@shrayolacrayon
Copy link

Thank you for opening @tahoward! We will review in the next few days and get back to you with any feedback we may have.

Copy link
Contributor

@benjsto benjsto left a comment

Choose a reason for hiding this comment

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

This is great - I tested it and it worked flawlessly. I just left a couple of minor naming comments. My only other feedback is that might be nice to also update the quickstart/docker-compose.yml to have this value in addition to the EMAIL_DOMAIN value.

Port int `envconfig:"PORT" default:"4180"`

EmailDomains []string `envconfig:"SSO_EMAIL_DOMAIN"`
EmailAddresses []string `envconfig:"SSO_EMAIL_ADDRESS"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I get that this is following the existing pattern with SSO_EMAIL_DOMAIN and EMAIL_DOMAIN but is there any case where these values would be different? Maybe it should just be EMAIL_ADDRESS for both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They both effectively serve the same purpose. We could combine them, however, we'd need a new parameter input for users to specify the intended validator to be used. Leaving them separated for now serves backwards comparability for those already using EMAIL_DOMAIN based validator.

@mreiferson mreiferson changed the title Add support for individual e-mail address authentication sso-auth: add provider for individual e-mail address authentication Nov 26, 2018
@mreiferson mreiferson added the enhancement New feature or request label Nov 26, 2018
@tahoward
Copy link
Contributor Author

Thanks for the review. I'll make some time this weekend to implement the suggestions.

@weeco
Copy link

weeco commented Dec 28, 2018

@tahoward Any update?

@xeor
Copy link

xeor commented Feb 15, 2019

Any chance of getting this one in soon?

@xeor
Copy link

xeor commented Feb 19, 2019

Did anyone push a version of the sso-dev image containing these changes to a public repo for testing?

@benjsto
Copy link
Contributor

benjsto commented Feb 20, 2019

Thanks for updating @tahoward !

@benjsto benjsto merged commit 683727d into buzzfeed:master Feb 25, 2019
@xeor
Copy link

xeor commented Feb 25, 2019

thanks for merging this!

I tried buzzfeed/sso-dev:683727d with the new option, and started wondering if this is missing from upstream_config? Shoulnt mail-adresses be able to be configured per upstream? Or have I misunderstood what it is trying to solve?

Shoulnt there be an allowed_emails, just like there is an allowed_groups in upstream-config

@victornoel
Copy link

@xeor I agree with your analysis yep, I think you are good for opening a new issue now this has been merged, too bad this wasn't detected earlier

@xeor
Copy link

xeor commented Feb 26, 2019

Weird that noone saw that :p Oh well...

@tahoward do you want to do this one as well? My go skills are non-existing yet :(

@benjsto
Copy link
Contributor

benjsto commented Feb 26, 2019

@xeor @victornoel : thanks for bringing this up (and creating a separate issue for it). I think it's perfectly valid to propose defining allowed_emails as a field per-upstream, but this PR correctly addresses the issue as written up initially as a counterpart to the EMAIL_DOMAIN global setting.

@victornoel
Copy link

@benjsto yep, sure, I think this should anyway be tackled in another PR since it's a bit different.

Actually I wonder why there isn't also an allowed_domain (or domains actually) at the route level by the way. What's the point of restricting access globally when you can do it at the route level... but this is a whole new story :) thanks for the good work @tahoward

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants