loginsrv

Unnamed repository; edit this file 'description' to name the repository.
git clone git@jamesshield.xyz:repos/loginsrv.git
Log | Files | Refs | README | LICENSE

commit f591a797c4813c76aa3a2a8ddbd72fab445c0063
parent 8d1a006065bb079cbe3289ab1e35adce236ad2a1
Author: Sebastian Mancke <sebastian.mancke@snabble.io>
Date:   Sat, 19 Jan 2019 17:32:47 +0100

Merge pull request #113 from tarent/issue-101

fixed corner cases in handling of JWT_SECRET paramter for caddy
Diffstat:
MREADME.md | 8++++----
Mcaddy/README.md | 15+++++++++++----
Mcaddy/setup.go | 20++++++++++++++------
Mcaddy/setup_test.go | 103++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Mlogin/config.go | 2+-
5 files changed, 132 insertions(+), 16 deletions(-)

diff --git a/README.md b/README.md @@ -22,7 +22,7 @@ It can be used as: * Standalone microservice * Docker container * Golang library -* [Caddy](http://caddyserver.com/) plugin +* [Caddy](http://caddyserver.com/) plugin. (See [caddy/README.md](./caddy/README.md) for details) ![](.screenshot.png) @@ -60,12 +60,12 @@ _Note for Caddy users_: Not all parameters are available in Caddy. See the table | -github | value | | X | OAuth config in the form: client_id=..,client_secret=..[,scope=..][,redirect_uri=..] | | -google | value | | X | OAuth config in the form: client_id=..,client_secret=..[,scope=..][,redirect_uri=..] | | -bitbucket | value | | X | OAuth config in the form: client_id=..,client_secret=..[,scope=..][,redirect_uri=..] | -| -facebook | value | | X | OAuth config in the form: client_id=..,client_secret=..,scope=email..[redirect_uri=..] | -| -gitlab | value | | X | OAuth config in the form: client_id=..,client_secret=..[,scope=..,][redirect_uri=..] +| -facebook | value | | X | OAuth config in the form: client_id=..,client_secret=..[,scope=..][,redirect_uri=..] | +| -gitlab | value | | X | OAuth config in the form: client_id=..,client_secret=..[,scope=..,][redirect_uri=..] | | -host | string | "localhost" | - | Host to listen on | | -htpasswd | value | | X | Htpasswd login backend opts: file=/path/to/pwdfile | | -jwt-expiry | go duration | 24h | X | Expiry duration for the JWT token, e.g. 2h or 3h30m | -| -jwt-secret | string | "random key" | X | Secret used to sign the JWT token | +| -jwt-secret | string | "random key" | X | Secret used to sign the JWT token. (See [caddy/README.md](./caddy/README.md) for details.) | | -jwt-algo | string | "HS512" | X | Signing algorithm to use (ES256, ES384, ES512, HS512, HS256, HS384, HS512) | | -log-level | string | "info" | - | Log level | | -login-path | string | "/login" | X | Path of the login resource | diff --git a/caddy/README.md b/caddy/README.md @@ -8,10 +8,17 @@ For a full documentation of loginsrv configuration and usage, visit the [loginsr A small demo can also be found in the [./demo](https://github.com/tarent/loginsrv/tree/master/caddy/demo) directory. -## Configuration -To be compatible with caddy-jwt, the jwt secret is taken from the environment variable `JWT_SECRET` -if such a variable is set. Otherwise, a random token is generated and set as environment variable JWT_SECRET, -so that caddy-jwt looks up the same shared secret. +## Configuration of `JWT_SECRET` +The jwt secret is taken from the environment variable `JWT_SECRET` if such a variable is set. +If a secret was configured in the directive config, this has higher priority and will be used over the environment variable in the case, +that both are set. This way, it is also possible to configure different secrets for multiple hosts. If no secret was set at all, +a random token is generated and used. + +To be compatible with caddy-jwt the secred is also written to the environment variable JWT_SECRET, if this variable was not set before. +This enables caddy-jwt to look up the same shared secret, even in the case of a random token. If the configuration uses differnt tokens +for different server blocks, only the first one will be stored in enviroment environment variable. You can't use a random key as the jwt-secret +and a custom one in the same caddyfile. If you want to have better control, of the integration with caddy-jwt, e.g. for multiple server blocks, +you should configure the jwt behaviour in caddy-jwt with the `secret` or `publickey` directives. ### Basic configuration Provide a login resource under /login, for user bob with password secret: diff --git a/caddy/setup.go b/caddy/setup.go @@ -48,12 +48,6 @@ func setup(c *caddy.Controller) error { config.LoginPath = path.Join(args[0], "/login") } - if e, isset := os.LookupEnv("JWT_SECRET"); isset { - config.JwtSecret = e - } else { - os.Setenv("JWT_SECRET", config.JwtSecret) - } - loginHandler, err := login.NewHandler(config) if err != nil { return err @@ -76,6 +70,7 @@ func parseConfig(c *caddy.Controller) (*login.Config, error) { fs := flag.NewFlagSet("loginsrv-config", flag.ContinueOnError) cfg.ConfigureFlagSet(fs) + secretProvidedByConfig := false for c.NextBlock() { // caddy prefers '_' in parameter names, // so we map them to the '-' from the command line flags @@ -95,7 +90,20 @@ func parseConfig(c *caddy.Controller) (*login.Config, error) { if err != nil { return cfg, fmt.Errorf("Invalid value for parameter %v: %v (%v:%v)", name, value, c.File(), c.Line()) } + + if name == "jwt-secret" { + secretProvidedByConfig = true + } } + secretFromEnv, secretFromEnvWasSetBefore := os.LookupEnv("JWT_SECRET") + if !secretProvidedByConfig && secretFromEnvWasSetBefore { + cfg.JwtSecret = secretFromEnv + } + if !secretFromEnvWasSetBefore { + // populate the secret to caddy.jwt, + // but do not change a environment variable, which somebody has set it. + os.Setenv("JWT_SECRET", cfg.JwtSecret) + } return cfg, nil } diff --git a/caddy/setup_test.go b/caddy/setup_test.go @@ -21,7 +21,6 @@ func TestSetup(t *testing.T) { for j, test := range []struct { input string shouldErr bool - config login.Config configCheck func(*testing.T, *login.Config) }{ { @@ -123,6 +122,108 @@ func TestSetup(t *testing.T) { } } +func TestSetup_CornerCasesJWTSecret(t *testing.T) { + + os.Setenv("JWT_SECRET", "jwtsecret") + + for j, test := range []struct { + description string + envInput string + config1 string + config2 string + expectedEnv string + expectedSecretConfig1 string + expectedSecretConfig2 string + }{ + { + description: "just use the environment", + envInput: "foo", + config1: `login { + simple bob=secret + }`, + config2: `login { + simple bob=secret + }`, + expectedEnv: "foo", + expectedSecretConfig1: "foo", + expectedSecretConfig2: "foo", + }, + { + description: "set variable using configs", + envInput: "", + config1: `login { + simple bob=secret + jwt_secret xxx + }`, + config2: `login { + simple bob=secret + jwt_secret yyy + }`, + expectedEnv: "xxx", + expectedSecretConfig1: "xxx", + expectedSecretConfig2: "yyy", + }, + { + description: "secret in env and configs was set", + envInput: "bli", + config1: `login { + simple bob=secret + jwt_secret bla + }`, + config2: `login { + simple bob=secret + jwt_secret blub + }`, + expectedEnv: "bli", // should not be touched + expectedSecretConfig1: "bla", + expectedSecretConfig2: "blub", + }, + { + description: "random default value", + envInput: "", + config1: `login { + simple bob=secret + }`, + config2: `login { + simple bob=secret + }`, + expectedEnv: login.DefaultConfig().JwtSecret, + expectedSecretConfig1: login.DefaultConfig().JwtSecret, + expectedSecretConfig2: login.DefaultConfig().JwtSecret, + }, + } { + t.Run(fmt.Sprintf("test %v %v", j, test.description), func(t *testing.T) { + if test.envInput == "" { + os.Unsetenv("JWT_SECRET") + } else { + os.Setenv("JWT_SECRET", test.envInput) + } + c1 := caddy.NewTestController("http", test.config1) + NoError(t, setup(c1)) + c2 := caddy.NewTestController("http", test.config2) + NoError(t, setup(c2)) + + mids1 := httpserver.GetConfig(c1).Middleware() + if len(mids1) == 0 { + t.Errorf("no middlewares created in test #%v", j) + return + } + middleware1 := mids1[len(mids1)-1](nil).(*CaddyHandler) + + mids2 := httpserver.GetConfig(c2).Middleware() + if len(mids2) == 0 { + t.Errorf("no middlewares created in test #%v", j) + return + } + middleware2 := mids2[len(mids2)-1](nil).(*CaddyHandler) + + Equal(t, test.expectedSecretConfig1, middleware1.config.JwtSecret) + Equal(t, test.expectedSecretConfig2, middleware2.config.JwtSecret) + Equal(t, test.expectedEnv, os.Getenv("JWT_SECRET")) + }) + } +} + func TestSetup_RelativeFiles(t *testing.T) { caddyfile := `loginsrv { template myTemplate.tpl diff --git a/login/config.go b/login/config.go @@ -184,7 +184,7 @@ func readConfig(f *flag.FlagSet, args []string) (*Config, error) { config := DefaultConfig() config.ConfigureFlagSet(f) - // prefer environment settings + // fist use the environment settings f.VisitAll(func(f *flag.Flag) { if val, isPresent := os.LookupEnv(envName(f.Name)); isPresent { f.Value.Set(val)