Repository navigation
Fix/expired user token in guest session - #3857
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 77 |
🟢 Coverage 80.33% diff coverage
Metric Results Coverage variation Report missing for 413082e1 Diff coverage ✅ 80.33% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (413082e) Report Missing Report Missing Report Missing Head commit (ae2116d) 93504 26861 28.73% Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3857) 61 49 80.33% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
rhafer
left a comment
There was a problem hiding this comment.
This breaks downloads for guest-users. For downloads we use the signedURL from the oc:downloadURL webdav property. Validating that signed url fails for guest users as the signed url validator needs to resolve to a "real" user.
Before the change downloads where working because the guest link authenticator ran before the signedurl authenticator and as the cookie was present the guestlink authenticator succeeded and the request never reached the signedurl authenticator.
Get the the order and interdependencies of those authenticators straight is somewhat tricky. I don't have a straightforward idea how to really fix this. I guess it would be good if we first documented what is actually supposed to happen when different types of credentials are available for a request. And which one is supposed to take precedence.
Also we should problably fix the signedurl authenticator to properly work with tokens signed for guest users 🤔
…enticator middelware
7e4df04 to
ae2116d
Compare
|
@maki5 With the bug being a P2 I think this should be merged. |
GuestLinkAuthenticatorafterPublicShareAuthenticatorandSignedURLAuthenticator, so valid credentials win.GuestLinkAuthenticatorto not handle credentials already handled by other authenticatorsSuppressAuthenticationChallengeto guest-only requests on guest pathscloses: #3721
might need a separate acceptance test @v-scharf