WIP - Documentation #21

synchronizing · 2021-02-19T18:01:02Z

Work in progress (WIP), please do not merge.

Effort to add documentation to the entire project. Any feedback is welcomed.

Todo

Remove root/src/<project> folder for the more conventional root/<project> format.
Finish documenting the rest of the codebase.
Complete Getting Started, Plug & Play, Settings, and Index documentation.
Update the README.md to reflect new documentation changes.
Add Contributing to documentation.
Adds automatic docs publishing.
Add documentation building commands to the Makefile.
Format code to be compliant with repos standard.
Add examples to the documentation.
- Best to likely have an /examples folder with the complete code, as well as the documentation page on the logic of the example code.

Note

This PR adds auto documentation publishing to the gh-pages, which automatically publishes the docs at <user>.github.io/aioauth/. See example here. This must be turned on in the repo settings.

Misc

Fix for #16, #22.
Update for #23.

synchronizing · 2021-02-19T18:04:05Z

.github/workflows/docs-publish.yml

+  push:
+    branches:
+      - master
+      - documentation # @todo Remove this before merging PR.


Reminder: This needs to be removed before PR is merged.

Added for the sake of building documentation on Github throughout the development of this pull request.

aliev · 2021-02-20T01:47:29Z

@synchronizing wow very impressive! many thanks for contributing!

aliev · 2021-02-20T13:26:09Z

docs/source/sections/understanding/plug_and_play.rst

+Plug-and-Play
+=============
+
+aioauth was designed to be as flexible as possible to allow developers to choose their own server framework, as well as database provider. Since aioauth is written as an asynchronous module it would be to the advantage of the developer to write their applications asynchronously.


great explanation!

@synchronizing wow very impressive! many thanks for contributing!

My pleasure on the contribution. I see the huge potential this library has, and I think documentation this early on would help flourishing the repo down the line. I have future plans to eventually helping out implementing OpenID Connect as well.

great explanation!

Thank you! I'm writing things as I go, so if you feel like anything is out-of-line, please don't hesitate to call it out/change it. I'll be working on this further come the this month (likely in a few weeks)- I got a huge deadline dropped on my table yesterday.

aioauth/config.py

synchronizing · 2021-02-24T18:50:04Z

@aliev Is there any reason for the use of Text over str throughout the project? Encounter it in models.py, and unsure if this is on purpose or not, as a few lines down str is used instead.

aliev · 2021-02-24T19:04:55Z

@synchronizing I had no apparent reason to replace str with Text. I think Text doesn't make sense for models, but maybe it makes sense to use it for Request / Response classes fields only? Note from the python docs:

Use Text to indicate that a value must contain a unicode string in a manner that is compatible with both Python 2 and Python 3:

in any case I see no problem in replacing Text with str. What do you think about that?

synchronizing · 2021-02-24T19:41:00Z

I agree with the replacement.

... but maybe it makes sense to use it for Request / Response classes fields only?

I agree with the use of unicode there - a lot of databases do support unicode, and I can certainly see the use for websites/providers that leverage different characters for their usernames/passwords. Will replace the str for username and password to Text.

synchronizing · 2021-02-25T00:10:21Z

Unsure why last commit is throwing line errors from flake8-black. Will investigate further at a later date. @aliev if you have any ideas from the top of your head, that would be wonderful, if not, I'll explore.

Running black -l 88 .. Locally, flake8 aioauth tests runs without error.

aliev · 2021-02-25T13:15:19Z

@synchronizing could you try black .? I tried on your branch:

(env) ➜  aioauth git:(documentation) black .
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/test_db.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/aioauth/server.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/aioauth/response_type.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/conftest.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/test_utils.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/classes.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/aioauth/utils.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/aioauth/grant_type.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/test_endpoint.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/test_request_validator.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/test_flow.py
reformatted /Users/ali/Projects/aliev/tmp/aioauth/tests/utils.py
All done! ✨ 🍰 ✨
12 files reformatted, 19 files left unchanged.
(env) ➜  aioauth git:(documentation) ✗ git status
On branch documentation
Your branch is up to date with 'origin/documentation'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   aioauth/grant_type.py
	modified:   aioauth/response_type.py
	modified:   aioauth/server.py
	modified:   aioauth/utils.py
	modified:   tests/classes.py
	modified:   tests/conftest.py
	modified:   tests/test_db.py
	modified:   tests/test_endpoint.py
	modified:   tests/test_flow.py
	modified:   tests/test_request_validator.py
	modified:   tests/test_utils.py
	modified:   tests/utils.py

no changes added to commit (use "git add" and/or "git commit -a")

synchronizing · 2021-02-25T15:14:29Z

@synchronizing could you try black .?

No cigar on my end. Unsure why.

10:13:16 at /Users/felipe/Desktop/aioauth 
  λ black .
All done! ✨ 🍰 ✨
31 files left unchanged.

10:13:18 at /Users/felipe/Desktop/aioauth 
  λ git status
On branch documentation
Your branch is up to date with 'origin/documentation'.

nothing to commit, working tree clean

Really weird. Will explore further, but your response was super helpful - likely some weird settings is getting applied for my Black formatter. Thank you.

aliev · 2021-02-25T16:03:09Z

@synchronizing hmm that's interesting. I cloned your fork, created virtual environment based on python 3.8:

python -mvenv env
source env/bin/activate

and did make dev-install.

this what flake8 command shows:

(env) ➜  aioauth git:(documentation) flake8 aioauth
aioauth/server.py:43:29: BLK100 Black would make changes.
aioauth/response_type.py:122:42: BLK100 Black would make changes.
aioauth/utils.py:167:33: BLK100 Black would make changes.
aioauth/grant_type.py:39:21: BLK100 Black would make changes.
(env) ➜  aioauth git:(documentation) flake8 tests  
tests/test_utils.py:76:60: BLK100 Black would make changes.
tests/conftest.py:105:36: BLK100 Black would make changes.
tests/test_db.py:35:29: BLK100 Black would make changes.
tests/test_flow.py:50:25: BLK100 Black would make changes.
tests/utils.py:37:49: BLK100 Black would make changes.
tests/test_endpoint.py:121:49: BLK100 Black would make changes.
tests/classes.py:31:14: BLK100 Black would make changes.
tests/test_request_validator.py:31:23: BLK100 Black would make changes.
(env) ➜  aioauth git:(documentation)

aliev · 2021-03-02T20:34:08Z

aioauth/grant_type.py

@@ -123,6 +143,18 @@ async def validate_request(self, request: Request) -> Client:


 class PasswordGrantType(GrantTypeBase):
+    """
+    The Password grant type is a way to exchange a user's credentials


@synchronizing should we remove the PasswordGrantType from the BaseAuthorizationServer by default?

I would advise against removing assuming I understand the question correctly - which would mean removing PasswordGrantType completely. Forgive me if you meant something else.

Password grant, while not recommended for every client, is the preferred method for those that separate their backend and frontend (also called, "first-party" apps, since the OAuth server owner also owns another application that leverages the server). In other words, if you have a FastAPI OAuth server and a seperate React frontend and mobile app, the only realistic way to have the user login is via the PasswordGrantType. To put it into perspective, you login to Twitter on their iOS app and React frontend via the PasswordGrantType, but login to (say) External Application X via their AuthorizationCodeGrantType, where the OAuth server asks "hey, you sure you want to authorize this app?" See here for more info.

While I haven't gotten to checking this just yet in relation to aioauth, one thing that authorization servers must have is the ability to specify which grant type a specific client has access to. Company A might have a OAuth server and frontend and mobile app that leverages the PasswordGrantType, while also allowing company B to use their OAuth server, but only have access to the AuthorizationCodeGrantType. See here for more info.

no, I did not mean complete deletion, as this is part of the oauth2 protocol. I thought about excluding it by default from BaseAuthorizationServer:

aioauth/src/aioauth/base/server.py

Lines 25 to 28 in 188699e

GrantType.TYPE_AUTHORIZATION_CODE: AuthorizationCodeGrantType,

GrantType.TYPE_CLIENT_CREDENTIALS: ClientCredentialsGrantType,

GrantType.TYPE_PASSWORD: PasswordGrantType,

GrantType.TYPE_REFRESH_TOKEN: RefreshTokenGrantType,

user can still enable it with the register method:

aioauth/src/aioauth/base/server.py

Lines 34 to 41 in 188699e

def register(

self,

endpoint_type: EndpointType,

server: Union[ResponseType, GrantType],

endpoint_cls: Union[Type[ResponseTypeBase], Type[GrantTypeBase]],

):

endpoint_dict = getattr(self, endpoint_type)

endpoint_dict[server] = endpoint_cls

but in any case, the user can also remove the PasswordGrantType on his own :)

Oh, I see!

I'm in favor, in such case. I haven't gotten to documenting the base server just yet, so I didn't know that function even existed. We can document this properly and ensure users are aware that there is the option, even though it's not recommended. Whatever the case, I wouldn't be making the modifications via this PR since it's exclusive to documentation, but I can add to the docs as soon as we make that change.

Indeed. Better to just document it and user will decide should it remove it or keep it

aliev · 2021-03-07T15:40:48Z

@synchronizing interesting, the build status is still waiting.

synchronizing · 2021-03-12T13:16:02Z

I think it is due to a bad merge from my end (master to documentation). Going to try to figure this out today and continue work on the PR.

synchronizing · 2021-03-12T15:00:42Z

Fixed the merge conflict, and Black situation. For future references, for whatever reason my system was defaulting to the latest version of Black (global install) despite activating the virtual environment. Running the env Black via relative path (./env/bin/black .) fixed all issues.

aliev · 2021-03-13T16:00:08Z

aioauth/models.py

+
+    code_challenge_method: Optional[CodeChallengeMethod] = None
+    """
+    Only used when `RFC 7636 <tools.ietf.org/html/rfc7636>`_,


@synchronizing
I think the link here must be also with http(s) prefix: https://synchronizing.github.io/aioauth/sections/documentation/models.html#aioauth.models.AuthorizationCode.code_challenge_method (otherwise it leads to the current path)

Sphinx wasn't playing nice at the time, and adding https:// was (surprisingly enough) breaking the hyperlink. Without it seemed to work, but I'll go ahead and leave this review open to go over this before finalizing things - I'll try to make https work.

codecov-io · 2021-03-18T20:12:21Z

Codecov Report

Merging #21 (0eea325) into master (188699e) will decrease coverage by 0.35%.
The diff coverage is 97.99%.

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
- Coverage   99.27%   98.92%   -0.36%     
==========================================
  Files          15       15              
  Lines         549      556       +7     
  Branches       56       56              
==========================================
+ Hits          545      550       +5     
- Misses          3        4       +1     
- Partials        1        2       +1

Impacted Files	Coverage Δ
aioauth/base/request_validator.py	`100.00% <ø> (ø)`
aioauth/base/server.py	`100.00% <ø> (ø)`
aioauth/errors.py	`100.00% <ø> (ø)`
aioauth/types.py	`100.00% <ø> (ø)`
aioauth/utils.py	`93.40% <93.40%> (ø)`
aioauth/base/database.py	`100.00% <100.00%> (ø)`
aioauth/config.py	`100.00% <100.00%> (ø)`
aioauth/constances.py	`100.00% <100.00%> (ø)`
aioauth/grant_type.py	`100.00% <100.00%> (ø)`
aioauth/models.py	`100.00% <100.00%> (ø)`
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 188699e...0eea325. Read the comment docs.

aliev · 2021-07-04T21:11:40Z

Makefile

@@ -48,8 +48,8 @@ clean-test: ## remove test and coverage artifacts
 	rm -fr .pytest_cache

 lint: ## check style with flake8
-	flake8 src/aioauth tests
-	pyright src/aioauth tests
+	flake8 aioauth tests


@synchronizing I'm thinking about removing the flake8 and black from the requirements and use only pre-commit.

Makes sense to me. I have to revisit the project soon- things took an unexpected turn in life. Whatever is changed in master I'll apply to this branch in time.

synchronizing · 2021-08-09T20:21:28Z

Closing this for the meantime. Planning on restarting this PR some point in the next few months.

If new PR is created regarding docs, any text written here can be utilized.

synchronizing added 7 commits February 18, 2021 13:50

Removed /src/ folder.

2d5de2b

Added documentation structure.

94e8780

Organized documentation.

9f1013a

config.py documentation.

0f97a1e

constances.py documentation.

4f74a3f

errors.py documentation.

236ec20

Added Github action to auto-publish documentation.

2f9dce5

synchronizing commented Feb 19, 2021

View reviewed changes

synchronizing added 2 commits February 19, 2021 13:13

Removed trailing white space from constances.py.

cc1c55a

Forces removal of old files in gh-pages.

dbdd1de

aliev reviewed Feb 20, 2021

View reviewed changes

synchronizing mentioned this pull request Feb 20, 2021

Pyright needs to be added to 'dev-install' make instruction. #22

Closed

synchronizing commented Feb 20, 2021

View reviewed changes

aioauth/config.py Show resolved Hide resolved

aliev added documentation Improvements or additions to documentation enhancement New feature or request labels Feb 21, 2021

synchronizing added 7 commits February 24, 2021 18:54

Updated config.py documentation.

d084be4

Updated constances.py documentation.

5cf91fd

Updated errors.py documentation.

8fc3afb

grant_type.py documentation.

9592d3e

models.py documentation.

546147c

Replaced NamedTuple's for SimpleNamespaces + fixed tests.

ce5f4f8

Ran Black on the existing codebase.

025a883

synchronizing mentioned this pull request Mar 1, 2021

Added badges: build status, coverage, license, pypi #23

Merged

synchronizing linked an issue Mar 1, 2021 that may be closed by this pull request

Pyright needs to be added to 'dev-install' make instruction. #22

Closed

aliev reviewed Mar 2, 2021

View reviewed changes

synchronizing added 4 commits March 5, 2021 12:03

Formatted using Black.

3a419b8

Added Makefile instruction to create docs & testing env.

9096a74

response_type.py documentation.

39567da

responses.py documentation.

0b7c22f

synchronizing added 3 commits March 12, 2021 09:52

Merge branch 'master' into documentation.

5aaed04

Fixed Makefile 'new-env' instruction.

568d5a3

Ran black formatting + flake8.

0e7bd70

synchronizing mentioned this pull request Mar 12, 2021

Remove PasswordGrantType from BaseAuthorizationServer. #25

Closed

2 tasks

aliev reviewed Mar 13, 2021

View reviewed changes

synchronizing mentioned this pull request Mar 18, 2021

OpenID Connect #26

Closed

synchronizing added 3 commits March 18, 2021 16:10

database.py documentation.

2a45f62

base/server.py documentation.

0ab0657

request_validator.py documentation.

0eea325

This was referenced Mar 29, 2021

refactor: move some business logic out of the db class #27

Merged

Project Restructure #28

Closed

aliev reviewed Jul 4, 2021

View reviewed changes

aliev mentioned this pull request Aug 8, 2021

Authorization Code flow with PKCE loses cookies between redirects #37

Closed

aliev mentioned this pull request Aug 9, 2021

Documentation #38

Merged

synchronizing closed this Aug 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP - Documentation #21

WIP - Documentation #21

synchronizing commented Feb 19, 2021 •

edited

Loading

synchronizing Feb 19, 2021 •

edited

Loading

aliev commented Feb 20, 2021

aliev Feb 20, 2021

synchronizing Feb 20, 2021 •

edited

Loading

synchronizing commented Feb 24, 2021 •

edited

Loading

aliev commented Feb 24, 2021

synchronizing commented Feb 24, 2021 •

edited

Loading

synchronizing commented Feb 25, 2021 •

edited

Loading

aliev commented Feb 25, 2021

synchronizing commented Feb 25, 2021

aliev commented Feb 25, 2021 •

edited

Loading

aliev Mar 2, 2021 •

edited

Loading

synchronizing Mar 3, 2021 •

edited

Loading

aliev Mar 5, 2021 •

edited

Loading

synchronizing Mar 5, 2021

aliev Mar 7, 2021

aliev commented Mar 7, 2021

synchronizing commented Mar 12, 2021 •

edited

Loading

synchronizing commented Mar 12, 2021 •

edited

Loading

aliev Mar 13, 2021 •

edited

Loading

synchronizing Mar 18, 2021

codecov-io commented Mar 18, 2021

aliev Jul 4, 2021

synchronizing Jul 10, 2021 •

edited

Loading

synchronizing commented Aug 9, 2021

	GrantType.TYPE_AUTHORIZATION_CODE: AuthorizationCodeGrantType,
	GrantType.TYPE_CLIENT_CREDENTIALS: ClientCredentialsGrantType,
	GrantType.TYPE_PASSWORD: PasswordGrantType,
	GrantType.TYPE_REFRESH_TOKEN: RefreshTokenGrantType,

	def register(
	self,
	endpoint_type: EndpointType,
	server: Union[ResponseType, GrantType],
	endpoint_cls: Union[Type[ResponseTypeBase], Type[GrantTypeBase]],
	):
	endpoint_dict = getattr(self, endpoint_type)
	endpoint_dict[server] = endpoint_cls

WIP - Documentation #21

WIP - Documentation #21

Conversation

synchronizing commented Feb 19, 2021 • edited Loading

Todo

Note

Misc

synchronizing Feb 19, 2021 • edited Loading

Choose a reason for hiding this comment

aliev commented Feb 20, 2021

aliev Feb 20, 2021

Choose a reason for hiding this comment

synchronizing Feb 20, 2021 • edited Loading

Choose a reason for hiding this comment

synchronizing commented Feb 24, 2021 • edited Loading

aliev commented Feb 24, 2021

synchronizing commented Feb 24, 2021 • edited Loading

synchronizing commented Feb 25, 2021 • edited Loading

aliev commented Feb 25, 2021

synchronizing commented Feb 25, 2021

aliev commented Feb 25, 2021 • edited Loading

aliev Mar 2, 2021 • edited Loading

Choose a reason for hiding this comment

synchronizing Mar 3, 2021 • edited Loading

Choose a reason for hiding this comment

aliev Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

synchronizing Mar 5, 2021

Choose a reason for hiding this comment

aliev Mar 7, 2021

Choose a reason for hiding this comment

aliev commented Mar 7, 2021

synchronizing commented Mar 12, 2021 • edited Loading

synchronizing commented Mar 12, 2021 • edited Loading

aliev Mar 13, 2021 • edited Loading

Choose a reason for hiding this comment

synchronizing Mar 18, 2021

Choose a reason for hiding this comment

codecov-io commented Mar 18, 2021

Codecov Report

aliev Jul 4, 2021

Choose a reason for hiding this comment

synchronizing Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

synchronizing commented Aug 9, 2021

synchronizing commented Feb 19, 2021 •

edited

Loading

synchronizing Feb 19, 2021 •

edited

Loading

synchronizing Feb 20, 2021 •

edited

Loading

synchronizing commented Feb 24, 2021 •

edited

Loading

synchronizing commented Feb 24, 2021 •

edited

Loading

synchronizing commented Feb 25, 2021 •

edited

Loading

aliev commented Feb 25, 2021 •

edited

Loading

aliev Mar 2, 2021 •

edited

Loading

synchronizing Mar 3, 2021 •

edited

Loading

aliev Mar 5, 2021 •

edited

Loading

synchronizing commented Mar 12, 2021 •

edited

Loading

synchronizing commented Mar 12, 2021 •

edited

Loading

aliev Mar 13, 2021 •

edited

Loading

synchronizing Jul 10, 2021 •

edited

Loading