Python generator: Add OAuth2_Password security feature. #404

tito97sp · 2024-03-10T20:21:06Z

This PR is part of #403

Not ready to merge.

Missing endpoint tests.

Note: this continues work started in #155

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
mvn clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/python*.
For Windows users, please run the script in Git BASH.

tito97sp · 2024-03-10T22:43:03Z

Error detected on scope_names definition for OAuth2PasswordBearer flow:

Traceback (most recent call last):
  File "/home/tito97sp/development/oauth2password/client/client_app_get_auth.py", line 42, in <module>
    api_response = api_instance.read_users_me_users_me__get()
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/paths/users_me/get/operation.py", line 90, in _read_users_me_users_me__get
    raw_response = self.api_client.call_api(
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/api_client.py", line 1036, in call_api
    self.update_params_for_auth(
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/api_client.py", line 1157, in update_params_for_auth
    security_scheme_instance.apply_auth(
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/security_schemes.py", line 373, in apply_auth
    raise exceptions.ApiValueError('scope_names are not defined and are required, define them')
oauth2password.exceptions.ApiValueError: scope_names are not defined and are required, define them

API used for testing:
oauth2password.json

Further work required.

@spacether how can I add this API for the endpoint tests??

spacether · 2024-03-11T20:53:53Z

Error detected on scope_names definition for OAuth2PasswordBearer flow:

Traceback (most recent call last):
  File "/home/tito97sp/development/oauth2password/client/client_app_get_auth.py", line 42, in <module>
    api_response = api_instance.read_users_me_users_me__get()
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/paths/users_me/get/operation.py", line 90, in _read_users_me_users_me__get
    raw_response = self.api_client.call_api(
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/api_client.py", line 1036, in call_api
    self.update_params_for_auth(
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/api_client.py", line 1157, in update_params_for_auth
    security_scheme_instance.apply_auth(
  File "/home/tito97sp/.local/share/Trash/files/python/src/oauth2password/security_schemes.py", line 373, in apply_auth
    raise exceptions.ApiValueError('scope_names are not defined and are required, define them')
oauth2password.exceptions.ApiValueError: scope_names are not defined and are required, define them

API used for testing: oauth2password.json

Further work required.

@spacether how can I add this API for the endpoint tests??

Can you edit this file: https://github.com/openapi-json-schema-tools/openapi-json-schema-generator/blob/master/src/test/resources/3_0/security.yaml
so include your oauth2 security and add it to a rout in that spec?

Then please regenerate the samples per the PR instructions (please do not delete them, they are useful)

mvn clean package 
./bin/generate-samples.sh

src/test/resources/3_0/security.yaml

src/test/resources/3_1/security.yaml

src/test/resources/3_0/security.yaml

spacether · 2024-03-17T17:47:25Z

@JoftheV how are you involved in this project? Pull reviews need to be feature complete and have all tests passing before they are approved and merged to master. This PR is not yet feature complete and tests are not yet passing. Are you a user who wants this feature? If so a comment saying that would be more helpful than an approval of the PR. I also just updated PR review privledges to limit it to read and higher members of this project.

samples/client/3_0_3_unit_test/python/pyproject.toml

spacether · 2024-03-25T23:46:42Z

src/main/java/org/openapijsonschematools/codegen/generators/DefaultGenerator.java

@@ -1368,6 +1373,10 @@ protected void updateModelForComposedSchema(CodegenSchema m, Schema schema, Stri
        }
    }

+    public LinkedHashSet<String> getOauthServerHostnames() {


How about returning null if oauthServerHostnames is empty?
Then you can simplify the call site to:
bundle.put("oauthServerHostnames", generator.getOauthServerHostnames());

Could you provide me more details about this? I do not get it completely.

You can update the code here to have:

if (oauthServerHostnames.isEmpty()) { return null; }

Then change the caller to remove the if statement and use bundle.put("oauthServerHostnames", generator.getOauthServerHostnames());

src/main/resources/python/security_schemes.hbs

samples/client/openapi_features/security/python/docs/paths/path_with_security_from_root/get.md

spacether

This PR is in good share but still needs some changes. Good work so far.
Remaining work

please add a test in the tests_manual folder verifying that when the password auth info is configured and a route with password auth is used, that the auth is applied to the request. One can see an example test that verifies a request here: https://github.com/openapi-json-schema-tools/openapi-json-schema-generator/blob/master/samples/client/petstore/python/tests_manual/test_paths/test_pet_pet_id/test_get.py#L54
please make the updates described in the comments that I have left

Once those updates have been done and merged and all tests are passing in CI, this will be ready to merge. Good job, you are almost there!

@spacether

@spacether review

…mentation errors in the tests.

tito97sp · 2024-03-27T14:01:48Z

Hi @spacether. I have added manual tests for oauth password in my last commit e46ba81 but still not working properly the tests. I ask for some support here to refine this tests.

Could your team help me here?

Test definition: test_get.py](https://github.com/openapi-json-schema-tools/openapi-json-schema-generator/pull/404/files/e46ba81b8c63ac1ad3091ea39bb5c77ad0e47773..92f013820df14846a2523e4858b3b0ffef7cf945#diff-69e9a5a27a59e261e79cc7cbec71aa0f2a6c3b616438879bd145424678c44e20)

tito97sp · 2024-03-27T14:04:09Z

...s/security/python/test_manual/test_paths/test_path_with_o_auth_password_security/test_get.py

+        # I need a first call for the /api/oauth/token endpoint to provide the access token
+        self.assert_pool_manager_request_called_with(
+            mock_request,
+            f'http://localhost:3000/api/oauth/token',
+            method='POST',
+            additional_headers={f'{access_token}': 'someAccessToken', 'token_type': 'Bearer'}
+        )
+
+        # I need a second call for the /pathWithOAuthPasswordSecurity endpoint to perform the actual request
+        # it should contain the access token
+        self.assert_pool_manager_request_called_with(
+            mock_request,
+            f'http://localhost:3000/pathWithOAuthPasswordSecurity',
+            method='GET',
+            additional_headers={'Authorization': f'Bearer {access_token}'}
+        )


Further work is required to fix this test. @spacether

Please patch the client.fetch_token call or the deeper requests call rather than patching the urllib3 call.
The deeper request call is at client.session.request I think, you will have to step through the code to see it. Or you can patch it like the authlib package does, see their tests here: https://github.com/lepture/authlib/blob/5ac468051098d544dd5bfad24f692ec1a6bc7ec1/tests/clients/test_requests/test_oauth2_session.py#L133

spacether · 2024-03-27T16:35:39Z

...client/openapi_features/security/python/src/this_package/configurations/api_configuration.py

@@ -96,6 +100,7 @@ class ApiConfiguration(object):
    :param security_index_info: path to security_index information
    :param server_info: the servers that can be used to make endpoint calls
    :param server_index_info: index to servers configuration
+    :param oauth_server_client_info: the oauth client_id and client_secret info


Please update the comment here. # the oauth client used for a specific oauth server the current comment is incorrect

spacether · 2024-05-13T21:39:24Z

Closing this due to inactivity. If someone wants to reopen it later, rebase it, and fix the needed tests, then I am open to having that merged.

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_1/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

spacether reviewed Mar 13, 2024

View reviewed changes

src/test/resources/3_0/security.yaml Outdated Show resolved Hide resolved

JoftheV approved these changes Mar 16, 2024

View reviewed changes

tito97sp marked this pull request as ready for review March 16, 2024 22:33

JoftheV approved these changes Mar 17, 2024

View reviewed changes

tito97sp force-pushed the feature/OAuth2-password-support branch from 9696449 to 46a5fd0 Compare March 17, 2024 13:16

JoftheV approved these changes Mar 17, 2024

View reviewed changes

Python generator: Add Oauth2 Password security feature

f28933b

tito97sp force-pushed the feature/OAuth2-password-support branch from 47ac23a to f28933b Compare March 24, 2024 19:54

tito97sp added 2 commits March 24, 2024 21:30

Solve mypy issue

872a800

Update tokenUrl in issue_7193.yaml and issue_7614.yaml

cc0b8d2

spacether reviewed Mar 25, 2024

View reviewed changes

samples/client/3_0_3_unit_test/python/pyproject.toml Outdated Show resolved Hide resolved

spacether reviewed Mar 25, 2024

View reviewed changes

src/main/resources/python/security_schemes.hbs Outdated Show resolved Hide resolved