Paths

Table of Contentst

Differential D5311

deposit.auth: Fix authentication failure corner case
ClosedPublic
Actions

Authored by ardumont on Mar 23 2021, 4:01 PM.

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T3166: Deploy deposit v0.13 in staging

Commits

rDDEP4140d97d13b6: deposit.auth: Fix authentication failure corner case

Summary

The error raised from keycloak can be less detailed than anticipated. This makes the
deposit server fails. So this commit fixes it by detailing only if it can. Delegating
such details to the newly craft swh.auth.keycloak.keycloak_error_message utility.

[1] https://sentry.softwareheritage.org/share/issue/ae84f6a49ded4adc84c3570020b527ee/

Related to T3166

Test Plan

tox

Diff Detail

Repository

rDDEP Push deposit

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 23 2021, 4:01 PM

ardumont added inline comments.Mar 23 2021, 4:03 PM

swh/deposit/tests/conftest.py
354	Previous version was too basic. The new test added made it appear as much. Thus the change.

Build is green

Patch application report for D5311 (id=19046)

Rebasing onto ba44a97373...

Current branch diff-target is up to date.

Changes applied before test

commit 0ab165351514944381f0c9968c60f454d4cca2f3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 23 15:59:31 2021 +0100

    deposit.auth: Fix authentication corner case
    
    It can happen that the error raised from keycloak is not detailed enough. This makes the
    deposit server not too happy about it. So this commit fixes it by detailing only if it
    can.

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

Harbormaster completed remote builds in B20121: Diff 19046.Mar 23 2021, 4:04 PM

ardumont requested review of this revision.Mar 23 2021, 4:04 PM

Rework commit message to reference task

ardumont edited the summary of this revision. (Show Details)Mar 23 2021, 4:05 PM

ardumont added a task: T3166: Deploy deposit v0.13 in staging.

Looks good to me, I think you should move the error message extraction code in swh-auth as this should be reusable in other code (swh-web for instance).

swh/deposit/auth.py
153–155	How about moving that code in an utility function in `swh-auth` ? def keycloak_error_message(keycloak_error: KeycloakError) -> str: ...

This revision is now accepted and ready to land.Mar 23 2021, 4:07 PM

ardumont edited the summary of this revision. (Show Details)Mar 23 2021, 4:08 PM

Build is green

Patch application report for D5311 (id=19047)

Rebasing onto ba44a97373...

Current branch diff-target is up to date.

Changes applied before test

commit 0d1f041a44c1586b05aafe156b8da2fc22d3ace1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 23 15:59:31 2021 +0100

    deposit.auth: Fix authentication corner case
    
    It can happen that the error raised from keycloak is not detailed enough. This makes the
    deposit server not too happy about it. So this commit fixes it by detailing only if it
    can.
    
    Related to T3166

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

Harbormaster completed remote builds in B20122: Diff 19047.Mar 23 2021, 4:08 PM

ardumont added inline comments.Mar 23 2021, 4:52 PM

swh/deposit/auth.py
153–155	yes, doing that now.

ardumont added inline comments.Mar 23 2021, 5:03 PM

swh/deposit/auth.py
153–155	D5312

ardumont retitled this revision from deposit.auth: Fix authentication corner case to deposit.auth: Fix authentication failure corner case.Mar 23 2021, 5:42 PM

ardumont edited the summary of this revision. (Show Details)

Rework commit message, diff description, and implementation to reuse swh.auth.keycloak
code.

Build is green

Patch application report for D5311 (id=19060)

Rebasing onto ba44a97373...

Current branch diff-target is up to date.

Changes applied before test

commit 4140d97d13b678ce50037e388ed663c8c43d85fb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 23 15:59:31 2021 +0100

    deposit.auth: Fix authentication failure corner case
    
    The error raised from keycloak can be less detailed than anticipated [1]. This makes the
    deposit server fails. So this commit fixes it by detailing only if it can. Delegating
    such details to the newly crafted swh.auth.keycloak.keycloak_error_message utility.
    
    [1] https://forge.softwareheritage.org/differential/revision/edit/5311/
    
    Related to T3166

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

Harbormaster completed remote builds in B20139: Diff 19060.Mar 23 2021, 5:46 PM

Closed by commit rDDEP4140d97d13b6: deposit.auth: Fix authentication failure corner case (authored by ardumont). · Explain WhyMar 23 2021, 5:52 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDDEP4140d97d13b6: deposit.auth: Fix authentication failure corner case.