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

feat: ignore other auth prefixes #4718

Merged
merged 15 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changesets/feat_lleadbet_ignore_other_auth_prefixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
### Ability to ignore other auth prefixes in the JWT plugin

You can now choose whether to ignore other header prefixes with the JWT plugin. Many applications will use the format of `Authorization: <scheme> <token>` and this will enable the use of other schemes within the `Authorization` header.

If the header prefix is an empty string, this option will be ignored.

You can configure this, such as:

```yaml title="router.yaml"
authentication:
router:
jwt:
header_name: authorization
header_value_prefix: "Bearer"
ignore_mismatched_prefix: true
```

In the above, the router will ignore `Authorization: Basic <token>`, but process requests with `Authorization: Bearer <token>` defined.

By [@lleadbet](https://github.com/lleadbet) in https://github.com/apollographql/router/pull/4718
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ expression: "&schema"
"default": "Bearer",
"type": "string"
},
"ignore_other_prefixes": {
"description": "Whether to ignore any mismatched prefixes",
"default": false,
"type": "boolean"
},
"jwks": {
"description": "List of JWKS used to verify tokens",
"type": "array",
Expand Down
11 changes: 9 additions & 2 deletions apollo-router/src/plugins/authentication/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ struct JWTConf {
/// Header value prefix
#[serde(default = "default_header_value_prefix")]
header_value_prefix: String,
/// Whether to ignore any mismatched prefixes
#[serde(default)]
ignore_other_prefixes: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we actually need an option. The router looks for the value with the prefix it needs, it ignores the rest, we still have the same behavior as before: if it does not find the value it wants, then the request will not be authenticated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that right? I thought we returned an InvalidPrefix message if we didn't recognise a prefix and that's what Lucas is changing. i.e.: what was an Invalid prefix fail will become, with this option enabled, continue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the important part is whether require_authentication is enabled

}

#[derive(Clone, Debug, Deserialize, JsonSchema)]
Expand Down Expand Up @@ -532,12 +535,16 @@ fn authenticate(

// Make sure the format of our message matches our expectations
// Technically, the spec is case sensitive, but let's accept
// case variations
//
// case variations. Furthermore, if the user has configured to ignore
// mismatched prefixes, we'll skip this check and instead do it in a
// later step.
let prefix_len = config.header_value_prefix.len();
if jwt_value.len() < prefix_len
|| !&jwt_value[..prefix_len].eq_ignore_ascii_case(&config.header_value_prefix)
{
if config.ignore_other_prefixes {
return ControlFlow::Continue(request);
}
return failure_message(
request.context,
AuthenticationError::InvalidPrefix(jwt_value_untrimmed, &config.header_value_prefix),
Expand Down
57 changes: 50 additions & 7 deletions apollo-router/src/plugins/authentication/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ fn create_an_url(filename: &str) -> String {
}

async fn build_a_default_test_harness() -> router::BoxCloneService {
build_a_test_harness(None, None, false).await
build_a_test_harness(None, None, false, false).await
}

async fn build_a_test_harness(
header_name: Option<String>,
header_value_prefix: Option<String>,
multiple_jwks: bool,
ignore_other_prefixes: bool,
) -> router::BoxCloneService {
// create a mock service we will use to test our plugin
let mut mock_service = test::MockSupergraphService::new();
Expand Down Expand Up @@ -110,6 +111,9 @@ async fn build_a_test_harness(
serde_json::Value::String(hp);
}

config["authentication"]["router"]["jwt"]["ignore_other_prefixes"] =
serde_json::Value::Bool(ignore_other_prefixes);

crate::TestHarness::builder()
.configuration_json(config)
.unwrap()
Expand Down Expand Up @@ -424,9 +428,44 @@ async fn it_accepts_when_auth_prefix_has_correct_format_and_valid_jwt() {
assert_eq!(expected_mock_response_data, response.data.as_ref().unwrap());
}

#[tokio::test]
async fn it_accepts_when_auth_prefix_does_not_match_config_and_is_ignored() {
let test_harness = build_a_test_harness(None, None, false, true).await;
// Let's create a request with our operation name
let request_with_appropriate_name = supergraph::Request::canned_builder()
.operation_name("me".to_string())
.header(http::header::AUTHORIZATION, "Basic dXNlcjpwYXNzd29yZA==")
.build()
.unwrap();

// ...And call our service stack with it
let mut service_response = test_harness
.oneshot(request_with_appropriate_name.try_into().unwrap())
.await
.unwrap();
let response: graphql::Response = serde_json::from_slice(
service_response
.next_response()
.await
.unwrap()
.unwrap()
.to_vec()
.as_slice(),
)
.unwrap();

assert_eq!(response.errors, vec![]);

assert_eq!(StatusCode::OK, service_response.response.status());

let expected_mock_response_data = "response created within the mock";
// with the expected message
assert_eq!(expected_mock_response_data, response.data.as_ref().unwrap());
}

#[tokio::test]
async fn it_accepts_when_auth_prefix_has_correct_format_multiple_jwks_and_valid_jwt() {
let test_harness = build_a_test_harness(None, None, true).await;
let test_harness = build_a_test_harness(None, None, true, false).await;

// Let's create a request with our operation name
let request_with_appropriate_name = supergraph::Request::canned_builder()
Expand Down Expand Up @@ -465,7 +504,8 @@ async fn it_accepts_when_auth_prefix_has_correct_format_multiple_jwks_and_valid_

#[tokio::test]
async fn it_accepts_when_auth_prefix_has_correct_format_and_valid_jwt_custom_auth() {
let test_harness = build_a_test_harness(Some("SOMETHING".to_string()), None, false).await;
let test_harness =
build_a_test_harness(Some("SOMETHING".to_string()), None, false, false).await;

// Let's create a request with our operation name
let request_with_appropriate_name = supergraph::Request::canned_builder()
Expand Down Expand Up @@ -504,7 +544,8 @@ async fn it_accepts_when_auth_prefix_has_correct_format_and_valid_jwt_custom_aut

#[tokio::test]
async fn it_accepts_when_auth_prefix_has_correct_format_and_valid_jwt_custom_prefix() {
let test_harness = build_a_test_harness(None, Some("SOMETHING".to_string()), false).await;
let test_harness =
build_a_test_harness(None, Some("SOMETHING".to_string()), false, false).await;

// Let's create a request with our operation name
let request_with_appropriate_name = supergraph::Request::canned_builder()
Expand Down Expand Up @@ -543,7 +584,7 @@ async fn it_accepts_when_auth_prefix_has_correct_format_and_valid_jwt_custom_pre

#[tokio::test]
async fn it_accepts_when_no_auth_prefix_and_valid_jwt_custom_prefix() {
let test_harness = build_a_test_harness(None, Some("".to_string()), false).await;
let test_harness = build_a_test_harness(None, Some("".to_string()), false, false).await;

// Let's create a request with our operation name
let request_with_appropriate_name = supergraph::Request::canned_builder()
Expand Down Expand Up @@ -583,13 +624,15 @@ async fn it_accepts_when_no_auth_prefix_and_valid_jwt_custom_prefix() {
#[tokio::test]
#[should_panic]
async fn it_panics_when_auth_prefix_has_correct_format_but_contains_whitespace() {
let _test_harness = build_a_test_harness(None, Some("SOMET HING".to_string()), false).await;
let _test_harness =
build_a_test_harness(None, Some("SOMET HING".to_string()), false, false).await;
}

#[tokio::test]
#[should_panic]
async fn it_panics_when_auth_prefix_has_correct_format_but_contains_trailing_whitespace() {
let _test_harness = build_a_test_harness(None, Some("SOMETHING ".to_string()), false).await;
let _test_harness =
build_a_test_harness(None, Some("SOMETHING ".to_string()), false, false).await;
}

async fn build_jwks_search_components() -> JwksManager {
Expand Down
17 changes: 17 additions & 0 deletions docs/source/configuration/authn-jwt.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,23 @@ The default value is `Bearer`.
</td>
</tr>

<tr>
<td style="min-width: 150px;">

##### `ignore_other_prefixes`

</td>
<td>

Whether to ignore other prefixes in the `Authorization` header. If set to `false`, or unspecified, the router will only accept tokens with the prefix specified in `header_value_prefix`. If set to `true`, the router will ignore any requests that don't start with the prefix specified in `header_value_prefix`.

If a header prefix is set to an empty string, this option is ignored.

The default value is `false`.

</td>
</tr>

</tbody>
</table>

Expand Down
Loading