Page MenuHomeSoftware Heritage
Differential D5192

swh.auth: Add coverage to the decode_token endpoint
ClosedPublic
Actions

Authored by ardumont on Mar 3 2021, 6:02 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3079: Boostrap swh-auth module for authentication
Commits
rDAUTH15fe363f3649: auth: Rework tests logic and add coverage to decode_token endpoint
Summary

This reworks the tests logic as well to reuse the way web tests are written.

This is a first step to actually use the mock class defined here as fixture for future modules which will depend on swh-auth (swh-web-client, swh-web and swh-deposit).

Related to T3079

Diff Detail

Repository
rDAUTH Common authentication libraries
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Mar 3 2021, 6:02 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 3 2021, 6:02 PM
swh-public-ci added a comment.Mar 3 2021, 6:03 PM

Build is green

Patch application report for D5192 (id=18570)

Rebasing onto a750b58718...

Current branch diff-target is up to date.
Changes applied before test
commit 6e640556e9a5583c4892597d25e28d024632d816
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Wed Mar 3 17:41:29 2021 +0100

    swh.auth: Add coverage to the decode_token endpoint
    
    This reworks the tests logic
    
    Related to T3079

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/1/ for more details.

Harbormaster completed remote builds in B19660: Diff 18570.Mar 3 2021, 6:03 PM
ardumont requested review of this revision.Mar 3 2021, 6:03 PM
ardumont edited the summary of this revision. (Show Details)Mar 3 2021, 6:15 PM
ardumont updated this revision to Diff 18573.Mar 3 2021, 6:16 PM

Drop unneeded docstring change

swh-public-ci added a comment.Mar 3 2021, 6:17 PM

Build is green

Patch application report for D5192 (id=18573)

Rebasing onto a750b58718...

Current branch diff-target is up to date.
Changes applied before test
commit b6e2bcd3c4687ca8fdf1ebc1878ad475211df213
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Wed Mar 3 17:41:29 2021 +0100

    swh.auth: Add coverage to the decode_token endpoint
    
    This reworks the tests logic
    
    Related to T3079

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/2/ for more details.

Harbormaster completed remote builds in B19663: Diff 18573.Mar 3 2021, 6:17 PM
ardumont retitled this revision from swh.auth: Add coverage to the decode_token endpoint to auth: Rework tests logic and add coverage to decode_token endpoint.Mar 3 2021, 6:17 PM
ardumont updated this revision to Diff 18574.Mar 3 2021, 6:17 PM
ardumont retitled this revision from auth: Rework tests logic and add coverage to decode_token endpoint to swh.auth: Add coverage to the decode_token endpoint.

rework commit message

swh-public-ci added a comment.Mar 3 2021, 6:18 PM

Build is green

Patch application report for D5192 (id=18574)

Rebasing onto a750b58718...

Current branch diff-target is up to date.
Changes applied before test
commit fe1d226e08893cc456bd449475e00066c52f453b
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Wed Mar 3 17:41:29 2021 +0100

    auth: Rework tests logic and add coverage to decode_token endpoint
    
    This reworks the tests logic as well to reuse the way web tests are written.
    
    This is a first step to actually use the mock class defined here as fixture for future
    modules which will depend on swh-auth (swh-web-client, swh-web and swh-deposit).
    
    Related to T3079

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/3/ for more details.

Harbormaster completed remote builds in B19664: Diff 18574.Mar 3 2021, 6:18 PM
ardumont planned changes to this revision.Mar 3 2021, 6:19 PM
anlambert added a subscriber: anlambert.Mar 3 2021, 6:28 PM
anlambert added inline comments.
swh/auth/tests/conftest.py
145–154

I think we should rather have a single keycloak fixture without factory

145–154

Ignore that comment, it should have been removed.

159

I am wondering if keycloak_mock would be a better fixture name.

160

How about adding a set_auth_success method to the KeycloackOpenIDConnectMock class ?

This way you need a single fixture and you simply have to call keycloak_mock.set_auth_success(False) at the beginning of a test.

ardumont marked 2 inline comments as done.Mar 3 2021, 6:30 PM
ardumont added inline comments.Mar 3 2021, 6:36 PM
swh/auth/tests/conftest.py
159

sure, why not.

160

sounds good.

ardumont added inline comments.Mar 3 2021, 6:39 PM
swh/auth/tests/conftest.py
145–154

i've been bitten by forgetting to remove comment prior to submit as well ;)

anlambert added inline comments.Mar 3 2021, 6:42 PM
swh/auth/tests/conftest.py
145–154

Yep, I messed up with the Cancel / Undo links.

ardumont updated this revision to Diff 18578.Mar 3 2021, 6:42 PM

Adapt according to suggestion

swh-public-ci added a comment.Mar 3 2021, 6:42 PM

Build is green

Patch application report for D5192 (id=18578)

Rebasing onto a750b58718...

Current branch diff-target is up to date.
Changes applied before test
commit 2d3a37d54c4eff3c1421f2e9b093cf1c6118cdb5
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Wed Mar 3 17:41:29 2021 +0100

    auth: Rework tests logic and add coverage to decode_token endpoint
    
    This reworks the tests logic as well to reuse the way web tests are written.
    
    This is a first step to actually use the mock class defined here as fixture for future
    modules which will depend on swh-auth (swh-web-client, swh-web and swh-deposit).
    
    Related to T3079

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/4/ for more details.

Harbormaster completed remote builds in B19668: Diff 18578.Mar 3 2021, 6:43 PM
anlambert accepted this revision.EditedMar 3 2021, 6:58 PM

I added a couple of nitpick comments but looks good to me otherwise.

swh/auth/tests/conftest.py
133

I guess you could pick a generic client id here now that code is not coupled to swh-web directly.

The factory will then be used in swh-web tests to instantiate the fixture with the swh-web client id.

swh/auth/tests/test_auth.py
47

Simulate failed authentication with Keycloak

This revision is now accepted and ready to land.Mar 3 2021, 6:58 PM
ardumont marked 2 inline comments as done.Mar 4 2021, 9:21 AM
ardumont added inline comments.
swh/auth/tests/conftest.py
133

right, i agree. The current data set is crafted to expect swh-web to be used.
The most annoying bit being the decoded-token...

I got lazy on building another one.
I'll check to change that but no promises ;)

ardumont added inline comments.Mar 4 2021, 9:37 AM
swh/auth/tests/conftest.py
133

middle-ground, i've:

  • made a default client_id in the factory definition
  • moved the creation of the keycloak-mock in the test_auth module, setting its client-id to swh-web

This way, no need to generate new data set.

ardumont updated this revision to Diff 18581.Mar 4 2021, 9:37 AM

Adapt towards what was suggested about the generic client id

swh-public-ci added a comment.Mar 4 2021, 9:38 AM

Build is green

Patch application report for D5192 (id=18581)

Rebasing onto a750b58718...

Current branch diff-target is up to date.
Changes applied before test
commit a9602f1dd229def084bfdaa58eb4694518ad44ea
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Wed Mar 3 17:41:29 2021 +0100

    auth: Rework tests logic and add coverage to decode_token endpoint
    
    This reworks the tests logic as well to reuse the way web tests are written.
    
    This is a first step to actually use the mock class defined here as fixture for future
    modules which will depend on swh-auth (swh-web-client, swh-web and swh-deposit).
    
    Related to T3079

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/6/ for more details.

Harbormaster completed remote builds in B19671: Diff 18581.Mar 4 2021, 9:38 AM
ardumont updated this revision to Diff 18582.Mar 4 2021, 9:40 AM

Explicit the client id relation in tests

swh-public-ci added a comment.Mar 4 2021, 9:41 AM

Build is green

Patch application report for D5192 (id=18582)

Rebasing onto a750b58718...

Current branch diff-target is up to date.
Changes applied before test
commit 15fe363f364976255dbcd981c41328d70a7d9a5d
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Wed Mar 3 17:41:29 2021 +0100

    auth: Rework tests logic and add coverage to decode_token endpoint
    
    This reworks the tests logic as well to reuse the way web tests are written.
    
    This is a first step to actually use the mock class defined here as fixture for future
    modules which will depend on swh-auth (swh-web-client, swh-web and swh-deposit).
    
    Related to T3079

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/7/ for more details.

Harbormaster completed remote builds in B19672: Diff 18582.Mar 4 2021, 9:41 AM
Closed by commit rDAUTH15fe363f3649: auth: Rework tests logic and add coverage to decode_token endpoint (authored by ardumont). · Explain WhyMar 4 2021, 9:44 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDAUTH15fe363f3649: auth: Rework tests logic and add coverage to decode_token endpoint.

Revision Contents
Changeset List

PathSize
swh/
auth/
tests/
160 lines
232 lines
89 lines

Diff 18583

View Options

swh/auth/tests/conftest.py

Loading...
View Options

swh/auth/tests/sample_data.py

Loading...
View Options

swh/auth/tests/test_auth.py

Loading...
About · Developer information · Legal