fixed corner cases in handling of JWT_SECRET paramter for caddy

this closes #101.

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                                                                 |

caddy/README.md

@@ -8,10 +8,15 @@ 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 ent environment variable.
 
 ### Basic configuration
 Provide a login resource under /login, for user bob with password secret:

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
 }

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

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)