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.

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

sso: support multiple identity providers #212

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Conversation

jphines
Copy link
Contributor

@jphines jphines commented Jun 12, 2019

Problem

We've been walking down the path to support multiple identity providers in several previous iterations. This last remaining stand is how to support configuration for multiple and different identity providers. The existing configuration structure is pretty inflexible and rigid when it comes to defining configuration, especially given our constraint to use environment variables.

We previously introduced viper to help solve some of these problems, but viper proved frustrating and lacking enough opinion to operate within. We roll that solution back here and instead use https://micro.mu/docs/go-config.html

Solution

We introduced a new configuration component based on https://micro.mu/docs/go-config.html to build this new, more complicated configuration mechanism. This is a ground-up re-write of the configuration mechanism we use, found separately in configuration.go.

Notes

This now looks like:

### session
SESSION COOKIE_NAME
SESSION_COOKIE_SECRET
SESSION_COOKIE_EXPIRE
SESSION_COOKIE_DOMAIN
SESSION_COOKIE_REFRESH
SESSION_COOKIE_SECURE
SESSION_COOKIE_HTTPONLY
SESSION_LIFETIME
SESSION_KEY


### client
CLIENT_PROXY_ID
CLIENT_PROXY_SECRET


### provider config for google
PROVIDER_*_TYPE
PROVIDER_*_SLUG
PROVIDER_*_CLIENT_ID
PROVIDER_*_CLIENT_SECRET
PROVIDER_*_SCOPE

### google specific 
PROVIDER_*_GOOGLE_CREDENTIALS
PROVIDER_*_GOOGLE_IMPERSONATE

### okta specific
PROVIDER_*_OKTA_URL
PROVIDER_*_OKTA_SERVER

### group refresh
PROVIDER_*_GROUPCACHE_INTERVAL_REFRESH
PROVIDER_*_GROUPCACHE_INTERVAL_PROVIDER


# server config
SERVER_SCHEME
SERVER_HOST
SERVER_PORT
SERVER_TIMEOUT_REQUEST
SERVER_TIMEOUT_WRITE
SERVER_TIMEOUT_READ


# authorize config
AUTHORIZE_PROXY_DOMAINS
AUTHORIZE_EMAIL_DOMAINS
AUTHORIZE_EMAIL_ADDRESSES


# metrics config 
METRICS_STATSD_PORT
METRICS_STATSD_HOST


# logging config
LOGGING_ENABLE
LOGGING_LEVEL

@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #212 into master will decrease coverage by 1.14%.
The diff coverage is 56.75%.

@@            Coverage Diff             @@
##           master     #212      +/-   ##
==========================================
- Coverage   62.34%   61.19%   -1.15%     
==========================================
  Files          49       50       +1     
  Lines        4002     4069      +67     
==========================================
- Hits         2495     2490       -5     
- Misses       1352     1392      +40     
- Partials      155      187      +32
Impacted Files Coverage Δ
internal/auth/providers/google_admin.go 0% <0%> (ø) ⬆️
internal/proxy/proxy.go 24.39% <0%> (-1.93%) ⬇️
internal/auth/providers/test_provider.go 0% <0%> (ø) ⬆️
internal/auth/authenticator.go 86.39% <100%> (-0.06%) ⬇️
internal/proxy/proxy_config.go 78.14% <100%> (+0.12%) ⬆️
internal/auth/configuration.go 49.73% <49.73%> (ø)
internal/auth/providers/google.go 59.2% <50%> (+1.16%) ⬆️
internal/auth/mux.go 75% <61.53%> (-0.72%) ⬇️
internal/auth/options.go 76.19% <73.58%> (-8.48%) ⬇️
internal/proxy/options.go 83.59% <80%> (-2%) ⬇️
... and 2 more

internal/auth/mux.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Jusshersmith Jusshersmith 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! 🔥

@jphines jphines merged commit 5904bbf into master Jun 18, 2019
@jphines jphines deleted the jhines-multiple-providers branch June 18, 2019 03:59
@Jusshersmith
Copy link
Contributor

Jusshersmith commented Jul 26, 2019

Below is a list of OLD_CONFIG -> NEW_CONFIG pairs to further aid and support anyone moving over to this new configuration set.

* in PROVIDER_*_TYPE and others represents a unique identifier grouping together a set of provider configs.

### SESSION
(NEW)                -> SESSION_COOKIE_NAME
COOKIE_SECRET        -> SESSION_COOKIE_SECRET
COOKIE_EXPIRE        -> SESSION_COOKIE_EXPIRE
COOKIE_DOMAIN        -> SESSION_COOKIE_DOMAIN
COOKIE_REFRESH       -> SESSION_COOKIE_REFRESH
COOKIE_SECURE        -> SESSION_COOKIE_SECURE
COOKIE_HTTP_ONLY     -> SESSION_COOKIE_HTTPONLY
SESSION_LIFETIME_TTL -> SESSION_LIFETIME
AUTH_CODE_SECRET     -> SESSION_KEY


### CLIENT
PROXY_CLIENT_ID     -> CLIENT_PROXY_ID
PROXY_CLIENT_SECRET -> CLIENT_PROXY_SECRET


### PROVIDER CONFIG FOR GOOGLE
(NEW)         -> PROVIDER_*_TYPE
(NEW)         -> PROVIDER_*_SLUG
CLIENT_ID     -> PROVIDER_*_CLIENT_ID
CLIENT_SECRET -> PROVIDER_*_CLIENT_SECRET
SCOPE         -> PROVIDER_*_SCOPE

### GOOGLE SPECIFIC 
GOOGLE_SERVICE_ACCOUNT_JSON -> PROVIDER_*_GOOGLE_CREDENTIALS
GOOGLE_ADMIN_EMAIL          -> PROVIDER_*_GOOGLE_IMPERSONATE

### OKTA SPECIFIC
OKTA_ORG_URL       -> PROVIDER_*_OKTA_URL
PROVIDER_SERVER_ID -> PROVIDER_*_OKTA_SERVER

### GROUP REFRESH
GROUPS_CACHE_REFRESH_TTL  -> PROVIDER_*_GROUPCACHE_INTERVAL_REFRESH
GROUPS_CACHE_PROVIDER_TTL -> PROVIDER_*_GROUPCACHE_INTERVAL_PROVIDER


# SERVER CONFIG
(NEW)             -> SERVER_SCHEME
HOST              -> SERVER_HOST
PORT              -> SERVER_PORT
REQUEST_TIMEOUT   -> SERVER_TIMEOUT_REQUEST
TCP_WRITE_TIMEOUT -> SERVER_TIMEOUT_WRITE
TCP_READ_TIMEOUT  -> SERVER_TIMEOUT_READ


# AUTHORIZE CONFIG
PROXY_ROOT_DOMAIN   -> AUTHORIZE_PROXY_DOMAINS
SSO_EMAIL_DOMAIN    -> AUTHORIZE_EMAIL_DOMAINS
SSO_EMAIL_ADDRESSES -> AUTHORIZE_EMAIL_ADDRESSES


# METRICS CONFIG 
STATSD_PORT -> METRICS_STATSD_PORT
STATSD_HOST -> METRICS_STATSD_HOST


# LOGGING CONFIG
REQUSEST_LOGGING -> LOGGING_ENABLE
(NEW)            -> LOGGING_LEVEL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants