-
Notifications
You must be signed in to change notification settings - Fork 33
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
Tencent Cloud support #99
base: main
Are you sure you want to change the base?
Conversation
@@ -82,7 +85,6 @@ A role must be specified when using this command through the --role flag. You ma | |||
outputType, _ := cmd.Flags().GetString(FlagOutputType) | |||
shellType, _ := cmd.Flags().GetString(FlagShellType) | |||
roleName, _ := cmd.Flags().GetString(FlagRoleName) | |||
cloudType, _ := cmd.Flags().GetString(FlagCloudType) |
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 can now infer the cloud type from the application in the cache. Giving the user the option to specify this doesn't make much sense - They'll almost certainly get it wrong. As such, this is removed.
It'll be removed from switch.go
as well.
cli/get.go
Outdated
@@ -166,7 +211,8 @@ A role must be specified when using this command through the --role flag. You ma | |||
ttl = config.TTL | |||
} | |||
|
|||
if cloudType == cloudAws { | |||
switch cloudType { |
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 can probably fold this switch
statement into the previous one if we're smart
return ApplicationTypeAWS, true | ||
} | ||
|
||
if strings.Contains(strings.ToLower(app.AppName), "tencent") { |
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.
This is likely to be changed depending on our implementation -- Okta doesn't have good first-class support for Tencent Cloud so they don't have a defined naming scheme, but we need a naming scheme to identify them.
Most likely, app.AppName will be set to something like keyconjurer_tencent
to be able to discern keyconjurer-enabled Tencent Cloud applications from ones that just take you to the console.
cli/get.go
Outdated
|
||
// TODO: Spin up a web server that listens for a SAML callback. | ||
|
||
// TODO: This only works for OSX. |
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 can use the work we have to implement this to add a new flag, -b/--open-browser
and add that to keyconjurer login
so users don't need to copy/paste.
b1d461e
to
3084bd1
Compare
3084bd1
to
7acfaa7
Compare
This will enable us to remove the old deprecated one
Otherwise our pipeline breaks
This PR re-implements Tencent Cloud support for KeyConjurer v2.
There are a few limitations to Tencent Cloud support which make it less-than-seamless to use:
--bypass-cache
flag cannot be used with Tencent applications; a Tencent Cloud application must be within the cache.