From 2b84bbd5d1b21a0730a264db3722f797ba791ea3 Mon Sep 17 00:00:00 2001 From: Luiz Fonseca Date: Fri, 3 Jan 2025 20:49:14 +0100 Subject: [PATCH 1/3] feat: add support to require 2FA from github users --- README.md | 7 +++ .../model/model.go | 20 +++---- .../router/oauth.go | 10 ++-- internal/pkg/jwt/jwt.go | 30 +++++++---- internal/pkg/jwt/jwt_test.go | 24 +++++++-- middleware_plugin.go | 54 ++++++++++++------- 6 files changed, 99 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 00782d8..353d2be 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ providing a more secure way for users to access protected routes. --label 'traefik.http.middlewares.whoami-github-oauth.plugin.github-oauth.apiBaseUrl=http://traefik-github-oauth-server' \ --label 'traefik.http.middlewares.whoami-github-oauth.plugin.github-oauth.whitelist.logins[0]=luizfonseca' \ --label 'traefik.http.middlewares.whoami-github-oauth.plugin.github-oauth.whitelist.teams[0]=827726' \ + --label 'traefik.http.middlewares.whoami-github-oauth.plugin.github-oauth.whitelist.twoFactorAuthRequired=true' \ --label 'traefik.http.routers.whoami.rule=Host(`whoami.example.com`)' \ --label 'traefik.http.routers.whoami.middlewares=whoami-github-oauth' \ traefik/whoami @@ -85,8 +86,14 @@ jwtSecretKey: optional_secret_key # The log level, defaults to info # Available values: debug, info, warn, error logLevel: info + # whitelist whitelist: + # When set to `true`, Github Users should have 2FA + # configured and set, otherwise they will be denied access + # Default is `false` + twoFactorAuthRequired: 'true' + # The list of GitHub user ids that are whitelisted to access the resources ids: - 996 diff --git a/internal/app/traefik-github-oauth-server/model/model.go b/internal/app/traefik-github-oauth-server/model/model.go index 31f69a6..623a5ff 100644 --- a/internal/app/traefik-github-oauth-server/model/model.go +++ b/internal/app/traefik-github-oauth-server/model/model.go @@ -19,10 +19,11 @@ type ResponseGenerateOAuthPageURL struct { } type ResponseGetAuthResult struct { - RedirectURI string `json:"redirect_uri"` - GitHubUserID string `json:"github_user_id"` - GitHubUserLogin string `json:"github_user_login"` - GithubTeamIDs []string `json:"github_team_ids"` + RedirectURI string `json:"redirect_uri"` + GitHubUserID string `json:"github_user_id"` + GitHubUserLogin string `json:"github_user_login"` + GithubTeamIDs []string `json:"github_team_ids"` + GithubUserTwoFactorAuth bool `json:"github_user_two_factor_auth"` } type ResponseError struct { @@ -30,9 +31,10 @@ type ResponseError struct { } type AuthRequest struct { - RedirectURI string `json:"redirect_uri"` - AuthURL string `json:"auth_url"` - GitHubUserID string `json:"github_user_id"` - GitHubUserLogin string `json:"github_user_login"` - GithubTeamIDs []string `json:"github_team_ids"` + RedirectURI string `json:"redirect_uri"` + AuthURL string `json:"auth_url"` + GitHubUserID string `json:"github_user_id"` + GitHubUserLogin string `json:"github_user_login"` + GithubTeamIDs []string `json:"github_team_ids"` + GithubUserTwoFactorAuth bool `json:"github_user_two_factor_auth"` } diff --git a/internal/app/traefik-github-oauth-server/router/oauth.go b/internal/app/traefik-github-oauth-server/router/oauth.go index cd7f5e2..054e1dc 100644 --- a/internal/app/traefik-github-oauth-server/router/oauth.go +++ b/internal/app/traefik-github-oauth-server/router/oauth.go @@ -109,6 +109,7 @@ func OauthRedirectHandler(app *server.App) http.HandlerFunc { authRequest.GitHubUserID = cast.ToString(githubData.User.GetID()) authRequest.GitHubUserLogin = githubData.User.GetLogin() + authRequest.GithubUserTwoFactorAuth = githubData.User.GetTwoFactorAuthentication() if authRequest.GithubTeamIDs != nil { var teamIDs []string @@ -169,10 +170,11 @@ func OauthAuthResultHandler(app *server.App) http.HandlerFunc { w, r, model.ResponseGetAuthResult{ - RedirectURI: authRequest.RedirectURI, - GitHubUserID: authRequest.GitHubUserID, - GitHubUserLogin: authRequest.GitHubUserLogin, - GithubTeamIDs: authRequest.GithubTeamIDs, + RedirectURI: authRequest.RedirectURI, + GitHubUserID: authRequest.GitHubUserID, + GitHubUserLogin: authRequest.GitHubUserLogin, + GithubTeamIDs: authRequest.GithubTeamIDs, + GithubUserTwoFactorAuth: authRequest.GithubUserTwoFactorAuth, }, ) } diff --git a/internal/pkg/jwt/jwt.go b/internal/pkg/jwt/jwt.go index 942ee3a..e525f69 100644 --- a/internal/pkg/jwt/jwt.go +++ b/internal/pkg/jwt/jwt.go @@ -7,16 +7,18 @@ import ( ) type PayloadUser struct { - Id string `json:"id"` - Login string `json:"login"` - Teams []string `json:"teams"` + Id string `json:"id"` + Login string `json:"login"` + Teams []string `json:"teams"` + TwoFactorEnabled bool `json:"two_factor_enabled"` } -func GenerateJwtTokenString(id string, login string, teamIds []string, key string) (string, error) { +func GenerateJwtTokenString(id string, login string, teamIds []string, key string, two_factor_enabled bool) (string, error) { token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ - "id": id, - "login": login, - "teams": teamIds, + "id": id, + "login": login, + "teams": teamIds, + "two_factor_enabled": two_factor_enabled, }) return token.SignedString([]byte(key)) } @@ -49,10 +51,18 @@ func ParseTokenString(tokenString, key string) (*PayloadUser, error) { } } + twoFactorEnabled := false + if claims["two_factor_enabled"] != nil { + if factorEnabled, ok := claims["two_factor_enabled"].(bool); ok { + twoFactorEnabled = factorEnabled + } + } + return &PayloadUser{ - Id: claims["id"].(string), - Login: claims["login"].(string), - Teams: teams, + Id: claims["id"].(string), + Login: claims["login"].(string), + Teams: teams, + TwoFactorEnabled: twoFactorEnabled, }, nil } else { return nil, fmt.Errorf("invalid token") diff --git a/internal/pkg/jwt/jwt_test.go b/internal/pkg/jwt/jwt_test.go index 0edeff3..2cd7b85 100644 --- a/internal/pkg/jwt/jwt_test.go +++ b/internal/pkg/jwt/jwt_test.go @@ -16,7 +16,7 @@ const ( func TestGenerateJwtTokenString(t *testing.T) { // execution - tokenString, err := GenerateJwtTokenString(id, login, testTeams, key) + tokenString, err := GenerateJwtTokenString(id, login, testTeams, key, false) // assertion assert.NoError(t, err) @@ -25,7 +25,7 @@ func TestGenerateJwtTokenString(t *testing.T) { func TestParseTokenString(t *testing.T) { // setup - tokenString, _ := GenerateJwtTokenString(id, login, testTeams, key) + tokenString, _ := GenerateJwtTokenString(id, login, testTeams, key, false) // execution payload, err := ParseTokenString(tokenString, key) @@ -39,7 +39,7 @@ func TestParseTokenString(t *testing.T) { func TestParseTokenString_EmptyTeams(t *testing.T) { // setup - tokenString, _ := GenerateJwtTokenString(id, login, []string{}, key) + tokenString, _ := GenerateJwtTokenString(id, login, []string{}, key, false) // execution payload, err := ParseTokenString(tokenString, key) @@ -53,7 +53,21 @@ func TestParseTokenString_EmptyTeams(t *testing.T) { func TestParseTokenString_NoTeams(t *testing.T) { // setup - tokenString, _ := GenerateJwtTokenString(id, login, nil, key) + tokenString, _ := GenerateJwtTokenString(id, login, nil, key, false) + + // execution + payload, err := ParseTokenString(tokenString, key) + + // assertion + assert.NoError(t, err) + assert.Equal(t, id, payload.Id) + assert.Equal(t, login, payload.Login) + assert.Equal(t, payload.Teams, []string{}) +} + +func TestParseTokenString_With2FAEnabled(t *testing.T) { + // setup + tokenString, _ := GenerateJwtTokenString(id, login, nil, key, true) // execution payload, err := ParseTokenString(tokenString, key) @@ -79,7 +93,7 @@ func TestParseTokenString_InvalidToken(t *testing.T) { func TestParseTokenString_InvalidKey(t *testing.T) { // setup - tokenString, _ := GenerateJwtTokenString(id, login, testTeams, key) + tokenString, _ := GenerateJwtTokenString(id, login, testTeams, key, false) invalidKey := "invalidkey" // execution diff --git a/middleware_plugin.go b/middleware_plugin.go index a357e8d..7f6989e 100644 --- a/middleware_plugin.go +++ b/middleware_plugin.go @@ -36,6 +36,8 @@ type Config struct { // ConfigWhitelist the middleware configuration whitelist. type ConfigWhitelist struct { + TwoFactorAuthRequired string `json:"two_factor_auth_required,omitempty"` + // Ids the GitHub user id list. Ids []string `json:"ids,omitempty"` // Logins the GitHub user login list. @@ -53,9 +55,10 @@ func CreateConfig() *Config { AuthPath: DefaultConfigAuthPath, JwtSecretKey: getRandomString32(), Whitelist: ConfigWhitelist{ - Ids: []string{}, - Logins: []string{}, - Teams: []string{}, + Ids: []string{}, + Logins: []string{}, + Teams: []string{}, + TwoFactorAuthRequired: "false", }, } } @@ -66,13 +69,14 @@ type TraefikGithubOauthMiddleware struct { next http.Handler name string - apiBaseUrl string - apiSecretKey string - authPath string - jwtSecretKey string - whitelistIdSet *strset.Set - whitelistLoginSet *strset.Set - whitelistTeamSet *strset.Set + apiBaseUrl string + apiSecretKey string + authPath string + jwtSecretKey string + whitelistIdSet *strset.Set + whitelistLoginSet *strset.Set + whitelistTeamSet *strset.Set + whitelistRequires2FA bool logger *log.Logger } @@ -96,13 +100,14 @@ func New(ctx context.Context, next http.Handler, config *Config, name string) (h next: next, name: name, - apiBaseUrl: baseUrl, - apiSecretKey: config.ApiSecretKey, - authPath: authPath, - jwtSecretKey: config.JwtSecretKey, - whitelistIdSet: strset.New(config.Whitelist.Ids...), - whitelistLoginSet: strset.New(config.Whitelist.Logins...), - whitelistTeamSet: strset.New(config.Whitelist.Teams...), + apiBaseUrl: baseUrl, + apiSecretKey: config.ApiSecretKey, + authPath: authPath, + jwtSecretKey: config.JwtSecretKey, + whitelistIdSet: strset.New(config.Whitelist.Ids...), + whitelistLoginSet: strset.New(config.Whitelist.Logins...), + whitelistTeamSet: strset.New(config.Whitelist.Teams...), + whitelistRequires2FA: config.Whitelist.TwoFactorAuthRequired == "true", logger: logger, }, nil @@ -133,6 +138,13 @@ func (middleware *TraefikGithubOauthMiddleware) handleRequest(rw http.ResponseWr return } + // Early check for 2FA -- if user is not whitelisted and 2FA is required, return 401 + if middleware.whitelistRequires2FA && !user.TwoFactorEnabled { + setNoCacheHeaders(rw) + http.Error(rw, "", http.StatusUnauthorized) + return + } + // If cookie is present, check if user is whitelisted // If nothing can be found, returns 404 as we don't want to leak information // But we log the error internally @@ -160,7 +172,13 @@ func (p TraefikGithubOauthMiddleware) handleAuthRequest(rw http.ResponseWriter, } // Generate JWTs - tokenString, err := jwt.GenerateJwtTokenString(result.GitHubUserID, result.GitHubUserLogin, result.GithubTeamIDs, p.jwtSecretKey) + tokenString, err := jwt.GenerateJwtTokenString( + result.GitHubUserID, + result.GitHubUserLogin, + result.GithubTeamIDs, + p.jwtSecretKey, + p.whitelistRequires2FA, + ) if err != nil { p.logger.Printf("Failed to generate JWT: %s", err.Error()) http.Error(rw, "", http.StatusInternalServerError) From d20c7a997e2b5bb030c4177e5b7e9f3644d33a60 Mon Sep 17 00:00:00 2001 From: Luiz Fonseca Date: Fri, 3 Jan 2025 20:50:02 +0100 Subject: [PATCH 2/3] chore: README update --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 353d2be..136dfba 100644 --- a/README.md +++ b/README.md @@ -89,8 +89,8 @@ logLevel: info # whitelist whitelist: - # When set to `true`, Github Users should have 2FA - # configured and set, otherwise they will be denied access + # When set to `true`, the middleware will check if the given user has 2FA + # configured, otherwise they will be denied access # Default is `false` twoFactorAuthRequired: 'true' From 7f2eca04e31e76cda42d986586be3b2cba52c30b Mon Sep 17 00:00:00 2001 From: Luiz Fonseca Date: Fri, 3 Jan 2025 20:55:04 +0100 Subject: [PATCH 3/3] chore: update test coverage --- internal/pkg/jwt/jwt_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/pkg/jwt/jwt_test.go b/internal/pkg/jwt/jwt_test.go index 2cd7b85..e0dd262 100644 --- a/internal/pkg/jwt/jwt_test.go +++ b/internal/pkg/jwt/jwt_test.go @@ -35,6 +35,7 @@ func TestParseTokenString(t *testing.T) { assert.Equal(t, id, payload.Id) assert.Equal(t, login, payload.Login) assert.Equal(t, testTeams, payload.Teams) + assert.False(t, payload.TwoFactorEnabled) } func TestParseTokenString_EmptyTeams(t *testing.T) { @@ -49,6 +50,7 @@ func TestParseTokenString_EmptyTeams(t *testing.T) { assert.Equal(t, id, payload.Id) assert.Equal(t, login, payload.Login) assert.Equal(t, payload.Teams, []string{}) + assert.False(t, payload.TwoFactorEnabled) } func TestParseTokenString_NoTeams(t *testing.T) { @@ -63,6 +65,7 @@ func TestParseTokenString_NoTeams(t *testing.T) { assert.Equal(t, id, payload.Id) assert.Equal(t, login, payload.Login) assert.Equal(t, payload.Teams, []string{}) + assert.False(t, payload.TwoFactorEnabled) } func TestParseTokenString_With2FAEnabled(t *testing.T) { @@ -77,6 +80,7 @@ func TestParseTokenString_With2FAEnabled(t *testing.T) { assert.Equal(t, id, payload.Id) assert.Equal(t, login, payload.Login) assert.Equal(t, payload.Teams, []string{}) + assert.True(t, payload.TwoFactorEnabled) } func TestParseTokenString_InvalidToken(t *testing.T) {