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

Add various cleanliness changes #114

Merged
merged 30 commits into from
Nov 21, 2024
Merged

Conversation

punmechanic
Copy link
Member

@punmechanic punmechanic commented Nov 13, 2024

  • Replace custom hand-rolled HTTP calls with the oidc/oauth2 packages
  • Move oauth2 handling code to oauth2/ package.
  • Use newer Go functions like slices.Contains instead of our hand-rolled functions
  • Simplify folder structure by moving commands outside of main package, and move main package to root folder so go run . works.
  • Migrate to Lambda function extension for Vault secret handling, so we don't need to handle it in the application.
  • Remove usage of the custom Riot Vault SDK, using the Hashicorp one instead.
  • Upgrade to v2 of the AWS SDK.
  • Pass HTTP client through context and remove go-rootcerts as it's no longer needed in newer Go versions.
  • Put binaries in bin/ instead of a custom directory.
  • Disable CGO.
  • Hide OIDC flags that no one actually uses.
  • Remove partial Tencent Cloud support. We will add this back, but we don't want regular usage of the application to panic.
  • Refactor environment variable writing so we rely less on string interpolation.

Resolves #111; using the OAuth2 library provides an OAuth2-related error when exchanging an access and id token for a web sso token, whereas previously this was silently ignored and would lead to a cryptic error when retrieving a SAML assertion.

punmechanic and others added 30 commits November 8, 2024 13:43
This should also fix a bug where non-HTTP 200 responses are not caught
and result in a cryptic error later in the exchange process
* Pass client through context. This would normally be frowned upon but
  we know we will only be using OAuth2's APIs to interact with Okta
  anyway.
* Implement oauth2.TokenSource on TokenSet, which removes the need to
  manually construct *oauth2.Token.
The config shouldn't "know" anything about the minutae of the token it
is receiving.
Takes the HandlePendingSession function much simpler
This was necessary due a bug in Go
(golang/go#14514) that was resolved in Go 1.8.
This will be reimplemented at some point in the future, but this has not
been working since 85f224a and attempting to use it results in a
run-time panic.
The UserInfo endpoint for Okta is standards-compliant, so we should use
a standards-compliant library to access it
Instead of using AWS authentication for Vault, users should be
instructed to use the Hashicorp Vault extension for AWS Lambda;
KeyConjurer's Lambda functions are not made aware of any authentication
details.

https://developer.hashicorp.com/vault/docs/platform/aws/lambda-extension
VAULT_SECRET_PATH conflicts with the Lambda extension.
Co-authored-by: dpantry <[email protected]>
Co-authored-by: ext-jmendes <[email protected]>
@punmechanic punmechanic merged commit 5ff3e2f into RiotGames:main Nov 21, 2024
2 checks passed
punmechanic added a commit that referenced this pull request Nov 21, 2024
5ff3e2f Add various cleanliness changes (#114)
2801c91 Update Windows and WSL usage instructions (#115)
REVERT: ada05bf Fix secret decoding
REVERT: 12476b7 Disable CGO
REVERT: 21e9853 Remove unused packages
REVERT: bfa112f Put binaries in bin/
REVERT: 1277d79 Correct key-value
REVERT: 0a57be4 Rename the environment variables
REVERT: 64fbd0b Do not use AWS auth for Vault
REVERT: d9824f5 Use official Hashicorp SDK for Vault
REVERT: edbdddb Use the v2 AWS SDK
REVERT: b8ebf58 Use newer versions of Go & aws-lambda-go
REVERT: 7c56a6f Use OIDC library for fetching UserInfo
REVERT: 1037f45 Remove partial Tencent Cloud support
REVERT: b812fcd Move oauth2 things of the command package
REVERT: 7cc87bd Remove unused package go-rootcerts
REVERT: 27f8b2a Move things around to reduce symbols in main package
REVERT: c693c4b Use a writer pattern for exporting environment variables
REVERT: bb55790 Refactor Get to have a struct, like Login
REVERT: ff3e035 Use slices.Contains() instead of our own custom function
REVERT: 7e86ece Use ClientContext() instead of the context key directly
REVERT: a0d1470 Move port handling out of the oauth2 file
REVERT: 5521578 Move socket creation into LoginCommand
REVERT: 62d20b6 Parse the ID token in LoginCommand, not in the Config
REVERT: f3ec1be Use OAuth2 package more heavily to simplify code
REVERT: 6c15e6b Use oauth2 package for the token exchange call

git-subtree-dir: keyconjurer-staging
git-subtree-split: 5ff3e2f
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.

Users with unauthorized sessions receive cryptic error when retrieving keys
1 participant