-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore: remove dex #1097
chore: remove dex #1097
Conversation
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
d03e6a2
to
4240c98
Compare
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
4240c98
to
7db94ef
Compare
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
) (*UserInfo, error) | ||
} | ||
|
||
type UserInfo struct { | ||
FirstName string `json:"first_name"` | ||
LastName string `json:"last_name"` | ||
Avatar string `json:"avatar"` | ||
Email string `json:"email"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we will support other Oauth services, I'm moving the generateToken
implementation to the API layer so the code can be reused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FirstName, LastName, and Avatar will be used in the new console. These values will be saved in the account_v2 table soon.
Issuer string `json:"issuer"` | ||
Audience string `json:"audience"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these two fields. Before, we used the values from dex. From now, we will set them in the values.yaml
oAuthConfig.Issuer, | ||
oAuthConfig.Audience, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the values form the config file.
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
) | ||
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{ | ||
s.logger.Error("Refresh token is invalid", zap.Any("refresh_token", refreshToken)) | ||
dt, err := auth.StatusUnauthenticated.WithDetails(&errdetails.LocalizedMessage{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the refresh token is generated by Bucketeer, we don't need to validate the redirectURL
and authType
.
If it fails when verifying, it will return an Unauthenticated
error.
string redirect_url = 2; | ||
AuthType type = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to validate these two fields because the refresh token is generated on the Bucketeer server.
rpc ExchangeBucketeerToken(ExchangeBucketeerTokenRequest) | ||
returns (ExchangeBucketeerTokenResponse); | ||
rpc RefreshBucketeerToken(RefreshBucketeerTokenRequest) | ||
returns (RefreshBucketeerTokenResponse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have renamed to use the old API name. Also, I have removed the old code.
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm removing this since we use the secret for the OAuth key.
string id_token = 5; | ||
} | ||
|
||
message IDTokenSubject { | ||
string user_id = 1; | ||
string conn_id = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed unused messages.
issuer string, | ||
audience string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two params didn't be used inside the NewAuthService
function.
return &authService{
issuer: issuer,
audience: audience,
signer: signer,
accountClient: accountClient,
verifier: verifier,
googleAuthenticator: google.NewAuthenticator(
&config.GoogleConfig, signer, logger,
),
opts: &options,
logger: logger,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thanks!
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🚀
LGTM!
Part of #405
Things done
This pull request includes several changes to the codebase, primarily focusing on the removal of certain OAuth configurations and dependencies from the backend and batch services, as well as the removal of the
dex
service. These changes are part of a larger effort to simplify the codebase and remove unnecessary dependencies.Removal of OAuth configurations:
manifests/bucketeer/charts/backend/templates/deployment.yaml
: Removed several OAuth-related environment variables from the backend service configuration. [1] [2]manifests/bucketeer/charts/backend/values.dev.yaml
: Removed several OAuth-related configurations from the values file for the backend service.manifests/bucketeer/charts/batch/templates/deployment.yaml
: Replaced theBUCKETEER_BATCH_OAUTH_CLIENT_ID
environment variable withBUCKETEER_BATCH_OAUTH_AUDIENCE
in the batch service configuration.manifests/bucketeer/charts/batch/values.dev.yaml
: Replaced theclientId
configuration withaudience
in the values file for the batch service.Removal of dependencies:
go.mod
: Removed several dependencies from the Go module file. [1] [2] [3]Removal of the
dex
service:manifests/bucketeer/charts/dex/.helmignore
,manifests/bucketeer/charts/dex/Chart.yaml
,manifests/bucketeer/charts/dex/templates/NOTES.txt
,manifests/bucketeer/charts/dex/templates/_helpers.tpl
,manifests/bucketeer/charts/dex/templates/cert-secret.yaml
,manifests/bucketeer/charts/dex/templates/configmap.yaml
,manifests/bucketeer/charts/dex/templates/deployment.yaml
: Removed all files related to thedex
service. [1] [2] [3] [4] [5] [6]Other changes:
Makefile
,hack/generate-service-token/README.md
,hack/generate-service-token/command.go
: Removed thesub
parameter from thegenerate-service-token
command and related documentation. [1] [2] [3] [4] [5]