Merge pull request #71 from istio-ecosystem/issue_69_state_and_nonce_…

…in_session

Move the state and nonce from an encrypted cookie into the session store

Makefile

@@ -39,7 +39,14 @@ run:
 	bazel run $(BAZEL_FLAGS) $(TARGET)
 
 test:
-	bazel test $(BAZEL_FLAGS) --strategy=TestRunner=standalone --test_output=all //test/...
+	bazel test $(BAZEL_FLAGS) --strategy=TestRunner=standalone --test_output=all --cache_test_results=no //test/...
+
+# Only run tests whose name matches a filter
+# Usage examples:
+#   make filter-test FILTER=*RetrieveToken*
+#   make filter-test FILTER=OidcFilterTest.*
+filter-test:
+	bazel test $(BAZEL_FLAGS) --strategy=TestRunner=standalone --test_output=all --cache_test_results=no //test/... --test_arg='--gtest_filter=$(FILTER)'
 
 coverage:
 	bazel coverage $(BAZEL_FLAGS) --instrumentation_filter=//src/ //...

bookinfo-example/config/authservice-configmap-template-for-authn-and-authz.yaml

@@ -46,7 +46,6 @@ data:
                 "client_id": "xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxx_CHANGE_ME",
                 "client_secret": "xxxxx-xxxx-xxx-xxx-xxx_CHANGE_ME",
                 "scopes": ["productpage.read", "reviews.read"],
-                "cryptor_secret": "xxx_CHANGE_ME",
                 "cookie_name_prefix": "productpage",
                 "id_token": {
                   "preamble": "Bearer",
@@ -56,7 +55,6 @@ data:
                   "preamble": "Bearer",
                   "header": "Authorization"
                 },
-                "timeout": 300,
                 "logout": {
                   "path": "/authservice_logout",
                   "redirect_to_uri": "https://<demo.example.change.me>/some/logout/path"

bookinfo-example/config/authservice-configmap-template-for-authn.yaml

@@ -46,13 +46,11 @@ data:
                 "client_id": "xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxx_CHANGE_ME",
                 "client_secret": "xxxxx-xxxx-xxx-xxx-xxx_CHANGE_ME",
                 "scopes": [],
-                "cryptor_secret": "xxx_CHANGE_ME",
                 "cookie_name_prefix": "productpage",
                 "id_token": {
                   "preamble": "Bearer",
                   "header": "Authorization"
                 },
-                "timeout": 300,
                 "logout": {
                   "path": "/authservice_logout",
                   "redirect_to_uri": "https://<demo.example.change.me>/some/logout/path"

config/oidc/config.proto

@@ -107,58 +107,45 @@ message OIDCConfig {
     // Required, but an empty array is allowed.
     repeated string scopes = 8;
 
-    // A secret used to derive cryptographic material for protecting cookies and other data.
-    // Can be any string.
-    // Required.
-    string cryptor_secret = 10 [(validate.rules).string.min_len = 1];
-
     // A unique identifier of the authservice's browser cookies. Can be any string.
     // Only needed when multiple services in the same domain are each protected by
     // their own authservice, in which case each service's authservice should have
     // a unique value to avoid cookie name conflicts.
     // Optional.
-    string cookie_name_prefix = 11;
+    string cookie_name_prefix = 9;
 
     // The configuration for adding ID Tokens as headers to requests forwarded to a service.
     // Required.
-    TokenConfig id_token = 12 [(validate.rules).message.required = true];
+    TokenConfig id_token = 10 [(validate.rules).message.required = true];
 
     // The configuration for adding Access Tokens as headers to requests forwarded to a service.
     // Optional.
-    TokenConfig access_token = 13;
-
-    // The number of seconds a user has to authenticate with the OIDC Provider before their
-    // authentication flow expires. The timer starts when an unauthenticated user visits
-    // a service protected by the authservice, keeps running while they are redirected to
-    // their OIDC Provider to log in, continues to run while they enter their
-    // username/password and potentially perform 2-factor authentication, and stops
-    // when the authservice receives the authcode from the OIDC provider's redirect.
-    // If it takes longer than the timeout for the authcode to be received, then the
-    // authcode will be rejected by the authservice causing the login to fail, even
-    // if the user successfully logged in to their OIDC Provider.
-    // Required.
-    uint32 timeout = 14 [(validate.rules).uint32.gte = 30];
+    TokenConfig access_token = 11;
 
     // When specified, the authservice will destroy the authservice session when a request is
     // made to the configured path.
     // Optional.
-    LogoutConfig logout = 15;
+    LogoutConfig logout = 12;
 
     // The Authservice associates obtained OIDC tokens with a session ID in a session store.
+    // It also stores some temporary information during the login process into the session store,
+    // which will be removed when the user finishes the login.
     // This configuration option sets the number of seconds since a user's session with the Authservice has started
     // until that session should expire.
     // When configured to `0`, which is the default value, the session will never timeout based on the time
     // that it was started, but can still timeout due to being idle.
     // When both `max_absolute_session_timeout` and `max_session_idle_timeout` are zero, then sessions will never
     // expire. These settings do not affect how quickly the OIDC tokens contained inside the user's session expire.
-    uint32 max_absolute_session_timeout = 16;
+    uint32 max_absolute_session_timeout = 13;
 
     // The Authservice associates obtained OIDC tokens with a session ID in a session store.
+    // It also stores some temporary information during the login process into the session store,
+    // which will be removed when the user finishes the login.
     // This configuration option sets the number of seconds since the most recent incoming request from that user
     // until the user's session with the Authservice should expire.
     // When configured to `0`, which is the default value, session expiration will not consider idle time,
     // but can still consider timeout based on maximum absolute time since added.
     // When both `max_absolute_session_timeout` and `max_session_idle_timeout` are zero, then sessions will never
     // expire. These settings do not affect how quickly the OIDC tokens contained inside the user's session expire.
-    uint32 max_session_idle_timeout = 17;
+    uint32 max_session_idle_timeout = 14;
 }

docs/README.md

@@ -90,14 +90,12 @@ The configuration of an OpenID Connect filter that can be used to retrieve ident
 | client_id | The OIDC client ID assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). Required. | string |
 | client_secret | The OIDC client secret assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). Required. | string |
 | scopes | Optional additional scopes passed to the OIDC Provider in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The `openid` scope is always sent to the OIDC Provider, and does not need to be specified here. Required, but an empty array is allowed. | (slice of) string |
-| cryptor_secret | A secret used to derive cryptographic material for protecting cookies and other data. Can be any string. Required. | string |
 | cookie_name_prefix | A unique identifier of the authservice's browser cookies. Can be any string. Only needed when multiple services in the same domain are each protected by their own authservice, in which case each service's authservice should have a unique value to avoid cookie name conflicts. Optional. | string |
 | id_token | The configuration for adding ID Tokens as headers to requests forwarded to a service. Required. | TokenConfig |
 | access_token | The configuration for adding Access Tokens as headers to requests forwarded to a service. Optional. | TokenConfig |
-| timeout | The number of seconds a user has to authenticate with the OIDC Provider before their authentication flow expires. The timer starts when an unauthenticated user visits a service protected by the authservice, keeps running while they are redirected to their OIDC Provider to log in, continues to run while they enter their username/password and potentially perform 2-factor authentication, and stops when the authservice receives the authcode from the OIDC provider's redirect. If it takes longer than the timeout for the authcode to be received, then the authcode will be rejected by the authservice causing the login to fail, even if the user successfully logged in to their OIDC Provider. Required. | uint32 |
 | logout | When specified, the authservice will destroy the authservice session when a request is made to the configured path. Optional. | LogoutConfig |
-| max_absolute_session_timeout | The Authservice associates obtained OIDC tokens with a session ID in a session store. This configuration option sets the number of seconds since a user's session with the Authservice has started until that session should expire. When configured to `0`, which is the default value, the session will never timeout based on the time that it was started, but can still timeout due to being idle. When both `max_absolute_session_timeout` and `max_session_idle_timeout` are zero, then sessions will never expire. These settings do not affect how quickly the OIDC tokens contained inside the user's session expire. | uint32 |
-| max_session_idle_timeout | The Authservice associates obtained OIDC tokens with a session ID in a session store. This configuration option sets the number of seconds since the most recent incoming request from that user until the user's session with the Authservice should expire. When configured to `0`, which is the default value, session expiration will not consider idle time, but can still consider timeout based on maximum absolute time since added. When both `max_absolute_session_timeout` and `max_session_idle_timeout` are zero, then sessions will never expire. These settings do not affect how quickly the OIDC tokens contained inside the user's session expire. | uint32 |
+| max_absolute_session_timeout | The Authservice associates obtained OIDC tokens with a session ID in a session store. It also stores some temporary information during the login process into the session store, which will be removed when the user finishes the login. This configuration option sets the number of seconds since a user's session with the Authservice has started until that session should expire. When configured to `0`, which is the default value, the session will never timeout based on the time that it was started, but can still timeout due to being idle. When both `max_absolute_session_timeout` and `max_session_idle_timeout` are zero, then sessions will never expire. These settings do not affect how quickly the OIDC tokens contained inside the user's session expire. | uint32 |
+| max_session_idle_timeout | The Authservice associates obtained OIDC tokens with a session ID in a session store. It also stores some temporary information during the login process into the session store, which will be removed when the user finishes the login. This configuration option sets the number of seconds since the most recent incoming request from that user until the user's session with the Authservice should expire. When configured to `0`, which is the default value, session expiration will not consider idle time, but can still consider timeout based on maximum absolute time since added. When both `max_absolute_session_timeout` and `max_session_idle_timeout` are zero, then sessions will never expire. These settings do not affect how quickly the OIDC tokens contained inside the user's session expire. | uint32 |
 
 
 

src/common/session/BUILD

@@ -3,58 +3,14 @@ load("//bazel:bazel.bzl", "xx_library")
 package(default_visibility = ["//visibility:public"])
 
 xx_library(
-    name = "hkdf",
+    name = "session_string_generator",
     srcs = [
-        "hkdf_deriver.cc",
+        "session_string_generator.cc",
     ],
     hdrs = [
-        "hkdf_deriver.h",
-    ],
-    deps = [
-        "@com_googlesource_boringssl//:crypto",
-    ],
-)
-
-xx_library(
-    name = "gcm_encryptor",
-    srcs = [
-        "gcm_encryptor.cc",
-    ],
-    hdrs = [
-        "gcm_encryptor.h",
-    ],
-    deps = [
-        "@com_github_abseil-cpp//absl/types:optional",
-        "@com_googlesource_boringssl//:crypto",
-    ],
-)
-
-xx_library(
-    name = "session_id_generator",
-    srcs = [
-        "session_id_generator.cc",
-    ],
-    hdrs = [
-        "session_id_generator.h",
+        "session_string_generator.h",
     ],
     deps = [
         "//src/common/utilities:random"
     ],
 )
-
-xx_library(
-    name = "token_encryptor",
-    srcs = [
-        "token_encryptor.cc",
-    ],
-    hdrs = [
-        "token_encryptor.h",
-    ],
-    deps = [
-        ":gcm_encryptor",
-        ":hkdf",
-        "//src/common/utilities:random",
-        "@com_github_abseil-cpp//absl/strings:strings",
-        "@com_googlesource_boringssl//:crypto",
-    ],
-)