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

Caching the OAuth tokens in Application Gateway #15924

Merged
merged 11 commits into from Nov 2, 2022

Conversation

mvshao
Copy link
Contributor

@mvshao mvshao commented Oct 27, 2022

Description

Changes proposed in this pull request:

  • OAuth tokens are stored per whole OAuth credentials -> clientID + clientSecret + authURL
  • mTLS OAuth tokens are stored per whole credentials -> clientID + sha256Certificate + sha256Key + authURL
  • minor improvement in the documentation regarding Application Gateway testing

Related issue(s)
For more information visit https://extra.secret.repository/kyma/backlog/issues/2774

@mvshao mvshao added the area/application-connector Issues or PRs related to application connectivity label Oct 27, 2022
@mvshao mvshao added this to the 2.8.2 milestone Oct 27, 2022
@kyma-bot kyma-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 27, 2022
@netlify
Copy link

netlify bot commented Oct 27, 2022

🥰 Documentation preview ready! 🥰

Name Link
🔨 Latest commit a7feb47
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/63622be30f562300081be2dc
😎 Deploy Preview https://deploy-preview-15924--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

majakurcius
majakurcius previously approved these changes Oct 27, 2022
@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 27, 2022
@franpog859 franpog859 self-assigned this Oct 31, 2022
Copy link
Contributor

@franpog859 franpog859 left a comment

Choose a reason for hiding this comment

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

Please, add also the test case for the normal OAuth, mentioned in the issue

Comment on lines +9 to +11
crt: {{ $files.Get "certs/invalid-ca/client.crt" | b64enc }}
key: {{ $files.Get "certs/invalid-ca/client.key" | b64enc }}
clientId: {{ "clientID" | b64enc }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in the tests, we can change all the clientIDs from someClientID... to just clientID, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can also change that

Comment on lines +9 to +10
crt: {{ $files.Get "certs/invalid-ca/client.crt" | b64enc }}
key: {{ $files.Get "certs/invalid-ca/client.key" | b64enc }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the ...negative... changed to the ...invalid-ca... there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same CA signs a positive certificate and a negative certificate. For this test, I generated certificates that are signed by other CA.

@kyma-bot kyma-bot removed the lgtm Looks good to me! label Oct 31, 2022
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Oct 31, 2022
@@ -5,5 +5,5 @@ metadata:
namespace: kyma-integration
type: Opaque
data:
clientId: {{ "bad id" | b64enc }}
clientId: {{ "clientID" | b64enc }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this one should not be changed :d It's the oauth-negative-incorrect-id case

@mvshao
Copy link
Contributor Author

mvshao commented Oct 31, 2022

/test pre-main-kyma-integration-k3d

@kyma-bot kyma-bot added lgtm Looks good to me! labels Nov 2, 2022
@kyma-bot kyma-bot removed lgtm Looks good to me! labels Nov 2, 2022
@mvshao
Copy link
Contributor Author

mvshao commented Nov 2, 2022

/test pre-main-kyma-integration-k3d

@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 2, 2022
@kyma-bot kyma-bot merged commit d965167 into kyma-project:main Nov 2, 2022
@mvshao mvshao deleted the oauth-cache branch November 2, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/application-connector Issues or PRs related to application connectivity hacktoberfest-accepted lgtm Looks good to me! size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants