Add JWT support as an option for sign in#1031
Conversation
|
Thanks for the contribution! Before we can merge this, we need @Bigbear325 to sign the Salesforce.com Contributor License Agreement. |
jacalata
left a comment
There was a problem hiding this comment.
I have an existing implementation of this that adds JWT as a sub-type of Credentials, which I think is a cleaner design that won't lead to a proliferation of sign_in_with_x methods. How do you feel about using that?
| parser.add_argument("--server", "-s", required=True, help="server address") | ||
| parser.add_argument("--site", "-t", help="site name") | ||
| auth = parser.add_mutually_exclusive_group(required=True) | ||
| auth = parser.add_mutually_exclusive_group(required=False) |
There was a problem hiding this comment.
JWT should be added as an option to this group
|
|
||
| # Only set this to False if you are running against a server you trust AND you know why the cert is broken | ||
| check_ssl_certificate = True | ||
| check_ssl_certificate = False |
There was a problem hiding this comment.
don't check in this change, we want people to use ssl by default
There was a problem hiding this comment.
good catch. I'll switch it to SSL. thank you
|
|
||
| # Make sure we use an updated version of the rest apis, and pass in our cert handling choice | ||
| server = TSC.Server(args.server, use_server_version=True, http_options={"verify": check_ssl_certificate}) | ||
| server = TSC.Server(args.server, use_server_version=True) |
There was a problem hiding this comment.
don't remove the http_options
| # This script demonstrates how to create a group using the Tableau | ||
| # Server Client. | ||
| # | ||
| # To run the script, you must have installed Python 3.6 or later. |
There was a problem hiding this comment.
Create group functionality is unrelated to JWT. Since this sample isn't going to include having someone create a jwt with the correct scope, I don't think it is useful to add.
| return server | ||
|
|
||
|
|
||
| def getSingleCredentialType(args): |
There was a problem hiding this comment.
all this is unnecessary if we add jwt to the argument group
| @@ -0,0 +1,13 @@ | |||
| class JsonWebTokenAuth(object): | |||
| def __init__(self, json_web_token, site_id=None): | |||
| self.json_web_token = json_web_token | |||
There was a problem hiding this comment.
why pass in a site id if it never gets used?
|
Hi Jac, if you already have a design for Jwt sign in, I'm up for it as well. Could I review your design as well? If all is good, we can close this CL. |
It's based on this: #1032 |
Since we start to support sign in Via JWT. need to change TSC as well for adopt this feature.
this CL: