Skip to content

Conversation

@RubenVerborgh
Copy link
Contributor

Fixes#558, the debug output for the same action is now:

 solid:ACL Check if ACL exists: https://localhost:8443/profile/card.acl +6s solid:ACL Permissions for (none): read +28ms solid:ACL Permissions for public: read +0ms solid:get /profile/card on localhost +1ms solid:get HEAD only +1ms 

debug('Authentication required')
thrownewHTTPError(401,`Access to ${this.resource} requires authorization`)
}else{
debug(`${mode} access denied for ${user}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a useful message, no?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.then(()=>next(),next)
constuserId=req.session.userId
req.acl.can(userId,mode)
.then(()=>next(),err=>{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: can replace () => next() with next

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't: the promise resolves to a non-falsy value (true), and the first parameter of next being truthy is interpreted as an error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️ 🤦‍♀️

Copy link
Contributor

@dan-fdan-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!!

@RubenVerborghRubenVerborgh merged commit e7d7ec3 into dz_oidcAug 31, 2017
@RubenVerborghRubenVerborgh deleted the fix/acl-less-debug branch August 31, 2017 20:23
RubenVerborgh added a commit that referenced this pull request Sep 3, 2017
* dz_oidc: (178 commits) Rename the idp option into multiuser (#570) Add a /public folder in new accounts (#569) Update Data Browser html file Remove debug overhead on ACL (#566) Fix requirement for additional verification logging in with WebID-TLS Add bootstrap.min.css.map to common/css/ Add current hash to redirect. (#562) Disable rejectUnauthorized on the WebID-TLS endpoint. (#561) Serve static common/ dir relative to __dirname Tweak account index page phrasing Remove solid:inbox from template Add support for external WebIDs registering with username & password Move RS options to oidc-auth-manager initialization Do not check for user header in oidc test Log whole error in error handler Expand error message for unverified web id Bump oidc-auth-manager dep to 0.12.0 Verify presence of test DNS entries. (#549) Expose WAC-Allow to browser clients. Remove async dependency. ...
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@RubenVerborgh@dan-f