CLI sending authorized request #4327
Conversation
bench/lib/context.go
Outdated
| @@ -41,6 +41,9 @@ const ( | |||
|
|
|||
| // CtxKeyCadenceClient is the name of the context key for the cadence client this cadence worker listens to | |||
| CtxKeyCadenceClient = ContextKey("ctxKeyCadenceClient") | |||
|
|
|||
| // CtxKeyJWT is the name of the context key for the JWT | |||
| CtxKeyJWT = ContextKey("ctxKeyJWT") | |||
There was a problem hiding this comment.
is bech/lib the right place to kave a context key ? I suspect it might be better in the cli folder but just placed it there because I found the other ones
There was a problem hiding this comment.
Yeah should not be here. You can put it int factory.go under cli/
common/config/config.go
Outdated
| @@ -63,7 +63,7 @@ type ( | |||
| // Blobstore is the config for setting up blobstore | |||
| Blobstore Blobstore `yaml:"blobstore"` | |||
| // Authorization is the config for setting up authorization | |||
| Authorization Authorization `yaml:authorization` | |||
| Authorization Authorization `yaml:"authorization""` | |||
There was a problem hiding this comment.
Seems like there is one more double quote at the end. Probably this cause the config doesn’t work
tools/cli/domainUtils.go
Outdated
| @@ -453,3 +453,7 @@ func getConfigDir(c *cli.Context) string { | |||
| } | |||
| return dirPath | |||
| } | |||
|
|
|||
| func getJWT(c *cli.Context) string { | |||
There was a problem hiding this comment.
Maybe in factory.go or others as this will be shared for all commands
tools/cli/flags.go
Outdated
| @@ -287,6 +288,10 @@ var flagsForExecution = []cli.Flag{ | |||
| Name: FlagRunIDWithAlias, | |||
| Usage: "RunID", | |||
| }, | |||
| cli.StringFlag{ | |||
| Name: FlagJWT, | |||
| Usage: "JWT token for authentication", | |||
There was a problem hiding this comment.
this is authorization not authentication :D
We have a different story for authentication: #4095
Pull Request Test Coverage Report for Build 91b798e2-7339-4a6d-aaa8-600c2837bafb
💛 - Coveralls |
config/development_oauth.yaml
Outdated
| maxJwtTTL: 600000000 | ||
| jwtCredentials: | ||
| algorithm: "RS256" | ||
| publicKey: "common/authorization/keytest.pub" |
There was a problem hiding this comment.
Let’s move the two files into a new folder under config/ , eg “credentials”. Because docker and home brew will automatically copy everything under config/ and will not keep the source code after binaries are built. That way users and test this feature with docker compose and homebrew with these test credentials
longquanzheng
changed the title
What changed?
--jwtflag added to CLIWhy?
Requested in this proposal
How did you test it?
Tested locally
and
Potential risks
Nothing as long as oauth file is not loaded
Release notes
When the whole proposal is implemented
Documentation Changes
When the whole proposal is implemented