sso-auth: add provider for individual e-mail address authentication by tahoward · Pull Request #113 · buzzfeed/sso

tahoward · 2018-11-10T20:19:12Z

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 · 2018-11-12T18:45:35Z

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

benjsto

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.

internal/proxy/options.go

benjsto · 2018-11-20T23:24:34Z

internal/auth/options.go

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

 	EmailDomains     []string `envconfig:"SSO_EMAIL_DOMAIN"`
+	EmailAddresses   []string `envconfig:"SSO_EMAIL_ADDRESS"`


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?

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.

tahoward · 2018-12-14T18:40:09Z

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

weeco · 2018-12-28T18:00:18Z

@tahoward Any update?

xeor · 2019-02-15T17:40:35Z

Any chance of getting this one in soon?

xeor · 2019-02-19T05:12:10Z

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

benjsto · 2019-02-20T03:25:08Z

Thanks for updating @tahoward !

xeor · 2019-02-25T22:34:37Z

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 · 2019-02-26T08:20:45Z

@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 · 2019-02-26T20:58:06Z

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 · 2019-02-26T20:59:00Z

@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 · 2019-02-26T21:26:37Z

@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

thoward-godaddy added 2 commits November 9, 2018 15:43

Add option for email addresses

24952fa

Add option for email addresses

8c088b3

shrayolacrayon assigned shrayolacrayon and benjsto and unassigned shrayolacrayon Nov 16, 2018

benjsto suggested changes Nov 20, 2018

View reviewed changes

mreiferson mentioned this pull request Nov 26, 2018

sso-auth: add support for individual e-mail address authentication #32

Closed

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 added the enhancement New feature or request label Nov 26, 2018

thoward-godaddy and others added 4 commits February 15, 2019 16:43

Change EMAIL_ADDRESS to EMAIL_ADDRESSES

686e7d7

Merge branch 'master' into email-addresses

2a9cd50

Fix line spacing for gofmt

a59a29e

Update docker-compose with comments

315c21e

benjsto approved these changes Feb 20, 2019

View reviewed changes

Merge branch 'master' into email-addresses

f9b08b4

benjsto merged commit 683727d into buzzfeed:master Feb 25, 2019

xeor mentioned this pull request Feb 26, 2019

A way to define allowed_emails in upstream-config #161

Closed

victornoel mentioned this pull request Feb 26, 2019

allowed_domain(s) in upstream config #162

Closed

StiviiK mentioned this pull request May 9, 2019

[incubator/buzzfeed-sso] Add ability to add whitelisted emails helm/charts#13648

Merged

4 tasks

Conversation

tahoward commented Nov 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Notes

Uh oh!

shrayolacrayon commented Nov 12, 2018

Uh oh!

benjsto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benjsto Nov 20, 2018

Choose a reason for hiding this comment

Uh oh!

tahoward Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

tahoward commented Dec 14, 2018

Uh oh!

weeco commented Dec 28, 2018

Uh oh!

xeor commented Feb 15, 2019

Uh oh!

xeor commented Feb 19, 2019

Uh oh!

benjsto commented Feb 20, 2019

Uh oh!

xeor commented Feb 25, 2019

Uh oh!

victornoel commented Feb 26, 2019

Uh oh!

xeor commented Feb 26, 2019

Uh oh!

benjsto commented Feb 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

victornoel commented Feb 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tahoward commented Nov 10, 2018 •

edited

Loading

benjsto commented Feb 26, 2019 •

edited

Loading