[Python] Conditionally set auth attributes

[sebastien-rosset]

Fix for #3844:

Do not add auth parameter in HTTP request if auth parameter is set None
Add attribute to control which auth schemes can be used.
Currently, the python client obtains the list of security schemes that have been specified in the OAS spec, and automatically adds the authentication attributes in the header, cookie or query. This happens even if the parameters are unset. For example, the "Authorization" header may be set with an empty Bearer token:

'Authorization': 'Bearer '
With this PR, the bearer access token is no longer set if its value is None.

Tests performed:

Use OAS spec that specifies more than one security scheme for a given operation, e.g. OAuth2 and cookie
Generate the Python SDK using the master branch
Write a Python client that sets the cookie but not the OAuth2 access token
Invoke the client and verifies both auth attributes have been set (causing auth failure)
Generate the Python SDK using code from this PR
Invoke the client again, verify the auth parameters are set conditionally.

PR checklist

• Read the contribution guidelines.
• If contributing template-only or documentation-only changes which will change sample output, build the project before.
• Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
• File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
• Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

[sebastien-rosset]

@taxpon@frol@mbohlool@cbornet@kenjones-cisco@tomplus@Jyhess @slash-arun @spacether

[spacether]

@sebastien-rosset how is this pull request different than your old one?

[sebastien-rosset]

@sebastien-rosset how is this pull request different than your old one?

Based on feedback from @wing328, I have made the following changes to avoid any breaking change for existing users:

• Retain the space after the api key prefix.
• Retain the "api_key" and "api_key_prefix" variables
• Don't add the OAuth2 access token as a input argument to the Configuration constructor, because it would be better to have a TokenSource. This could be done in a separate PR.

[sebastien-rosset]

There seems to be an unrelated build issue:

Loading composer repositories with package information
Failed to clone the git@github.com:ybelenko/slim-token-authentication.git repository, try running in interactive mode so that you can enter your GitHub credentials

[spacether]

@wing328 why should we keep the api_key_prefix data in a separate dict when that data is storing info about an api_key? If we store it there it would be a breaking change with fallback and it reduces the number or dicts that we use from two to one. That way questions like:

• what value should I use for an api_key
• what prefix is used when sending this key
  Can all be answered in the one api_key dict

[sebastien-rosset]

@wing328 why should we keep the api_key_prefix data in a separate dict when that data is storing info about an api_key? If we store it there it would be a breaking change with fallback and it reduces the number or dicts that we use from two to one. That way questions like:

• what value should I use for an api_key
• what prefix is used when sending this key
  Can all be answered in the one api_key dict

My understanding is that we should split in two separate PRs: one that can go in master without breaking the backward compatibility (because master will be used as the basis for the next minor revision), and another PR that should go into the next release that can potentially break backward compatibility.

[spacether]

@wing328 why should we keep the api_key_prefix data in a separate dict when that data is storing info about an api_key? If we store it there it would be a breaking change with fallback and it reduces the number or dicts that we use from two to one. That way questions like:

• what value should I use for an api_key
• what prefix is used when sending this key
  Can all be answered in the one api_key dict

My understanding is that we should split in two separate PRs: one that can go in master without breaking the backward compatibility (because master will be used as the basis for the next minor revision), and another PR that should go into the next release that can potentially break backward compatibility.

Ah understood. Thanks for your explanation.

[sebastien-rosset]

Unrelated network error during build

[sebastien-rosset]

Build failure due to unrelated transient network issue.

[spacether]

This looks good. Thanks for this PR @sebastien-rosset !

[spacether]

Did we removed a feature where
auth_settings['value'] stored a specific auth data store?

[sebastien-rosset]

Did we removed a feature where
auth_settings['value'] stored a specific auth data store?

No, I don't think so. I think the problem is happening because @martyzz1 has a configuration.mustache template based on the old version. In the new configuration.mustache template, the code makes sure to skip auth_settings items if the corresponding auth parameters are not set in the Configuration class.

[sebastien-rosset]

Did we removed a feature where
auth_settings['value'] stored a specific auth data store?

Just to make sure, are you referring to @martyzz1's issue, or something else?

[spacether]

Did we removed a feature where
auth_settings['value'] stored a specific auth data store?

Just to make sure, are you referring to @martyzz1's issue, or something else?

I was referring to this PR but upon closer reading of it, I don't think that we removed a feature here. We're okay.

[martyzz1]

From @sebastien-rosset 's detailed response - and knowing the changes I made to my configuration.mustache - I'd agree that no features are likely to have been removed...

[sebastien-rosset]

From @sebastien-rosset 's detailed response - and knowing the changes I made to my configuration.mustache - I'd agree that no features are likely to have been removed...

Thank you for your response. I will mark the issue resolved for now, feel free to reopen if needed.