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

[TT-9671] Add jwks timeout #5382

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[TT-9671] Add jwks timeout #5382

wants to merge 5 commits into from

Conversation

zalbiraw
Copy link
Member

@zalbiraw zalbiraw commented Aug 2, 2023

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@zalbiraw zalbiraw changed the title Add jwks timeout [TT-9671] Add jwks timeout Aug 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

PR Analysis

  • 🎯 Main theme: Adding a timeout for JWT JWKs cache
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • Focused PR: Yes, the PR is focused as it is clear from the title, description and the changes made that the PR is about adding a timeout for JWT JWKs cache.
  • 🔒 Security concerns: No, the changes made in this PR do not introduce any apparent security concerns as it is mainly about adding a timeout for JWT JWKs cache.

PR Feedback

  • 💡 General PR suggestions: The PR is well-structured and the changes are clear. However, it would be beneficial to add tests to ensure the new timeout functionality works as expected. Also, it would be helpful to provide more context in the PR description about why this change is necessary and what problem it solves.

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve: Suggest improvements to the code in the PR.
/ask <QUESTION>: Pose a question about the PR.

To edit any configuration parameter from 'configuration.toml', add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Aug 2, 2023

API tests result: success
Branch used: refs/pull/5382/merge
Commit:
Triggered by: pull_request (@zalbiraw)
Execution page

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

API Changes

--- prev.txt	2024-05-06 17:30:22.097364802 +0000
+++ current.txt	2024-05-06 17:30:19.021384225 +0000
@@ -188,6 +188,9 @@
         "jwt_source": {
             "type": "string"
         },
+        "jwt_jwks_cache_timeout": {
+            "type": "number"
+        },
         "jwt_identity_base_field": {
             "type": "string"
         },
@@ -966,6 +969,7 @@
 	JWTIssuedAtValidationSkew            uint64                 `bson:"jwt_issued_at_validation_skew" json:"jwt_issued_at_validation_skew"`
 	JWTExpiresAtValidationSkew           uint64                 `bson:"jwt_expires_at_validation_skew" json:"jwt_expires_at_validation_skew"`
 	JWTNotBeforeValidationSkew           uint64                 `bson:"jwt_not_before_validation_skew" json:"jwt_not_before_validation_skew"`
+	JWTJWKSCacheTimeout                  int64                  `bson:"jwt_jwks_cache_timeout" json:"jwt_jwks_cache_timeout"`
 	JWTSkipKid                           bool                   `bson:"jwt_skip_kid" json:"jwt_skip_kid"`
 	Scopes                               Scopes                 `bson:"scopes" json:"scopes,omitempty"`
 	IDPClientIDMappingDisabled           bool                   `bson:"idp_client_id_mapping_disabled" json:"idp_client_id_mapping_disabled"`

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 7, 2023

API tests result: success
Branch used: refs/pull/5382/merge
Commit: 3d6ea8b
Triggered by: pull_request (@zalbiraw)
Execution page

@zalbiraw zalbiraw force-pushed the tt-9671-param-jwks-cache branch from 3d6ea8b to 9a08b47 Compare September 15, 2023 19:06
@github-actions
Copy link
Contributor

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@buger
Copy link
Member

buger commented Sep 15, 2023

API tests result: success
Branch used: refs/pull/5382/merge
Commit: 9a08b47
Triggered by: pull_request (@zalbiraw)
Execution page

@github-actions
Copy link
Contributor

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@buger
Copy link
Member

buger commented Sep 15, 2023

API tests result: success
Branch used: refs/pull/5382/merge
Commit: 1052563
Triggered by: pull_request (@zalbiraw)
Execution page

@github-actions
Copy link
Contributor

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@buger
Copy link
Member

buger commented Sep 15, 2023

API tests result: failure 🚫
Branch used: refs/pull/5382/merge
Commit: 12d7479
Triggered by: pull_request (@zalbiraw)
Execution page

Copy link
Contributor

github-actions bot commented May 6, 2024

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

1 similar comment
Copy link
Contributor

github-actions bot commented May 6, 2024

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants