Skip to content

Add support for external WebIDs registering with username & password - #553

Merged
dmitrizagidulin merged 3 commits into
dz_oidcfrom
external-webids-username-password
Aug 30, 2017
Merged

dmitrizagidulin merged 3 commits into
dz_oidcfrom
external-webids-username-password

Conversation

@dmitrizagidulin

Copy link
Copy Markdown
Contributor

(Note: changes default templates & views, be sure to clear config/views & config/templates before testing.)

@RubenVerborgh

Copy link
Copy Markdown
Contributor

Logging in without an account

This server is not the authorized provider for Web ID https://ruben.verborgh.org/profile/#me. See https://github.com/solid/webid-oidc-spec#authorized-oidc-issuer-discovery

Nice!

Registering with a garbage WebID

Tried registering with "xyz" as a WebID, it let me… Might be good to add a URI check for people who don't know what WebID is (and might fill out things like their username, "none", etc.).

Registering with an actual WebID

Works fine; I can login with username as well as WebID, and I get the proper error message when the triple is not set in my profile.

@RubenVerborgh RubenVerborgh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just a couple of minor thoughts, but no blockers.

</p>
<p>
Your Web ID is:<br />
Web ID:<br />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to make this more descriptive? As in The owner of this page/space is {{webId}}? There was some confusion in the past with "Your Web ID is", and this removes some confusion, but maybe not all.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good idea

solid:account </> ; # link to the account uri
pim:storage </> ; # root storage

solid:inbox </inbox/> ;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need both?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, we do not.

*/
function handler (err, req, res, next) {
debug('Error page because of ' + err.message)
debug('Error page because of:', err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can debug handle a second argument?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's pretty great, like console.log

if (userConfig.username) {
if (userConfig.externalWebId && !userConfig.localAccountId) {
// External Web ID exists, derive the local account id from username
userConfig.localAccountId = this.accountWebIdFor(userConfig.username)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this means I can automatically get a username assigned?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sort of. the localAccountId just maps from the local server's username-based url to the external Web ID

@dmitrizagidulin

Copy link
Copy Markdown
Contributor Author

Whoops, I forgot this PR was still unmerged.

@dmitrizagidulin
dmitrizagidulin force-pushed the external-webids-username-password branch from 78e7138 to c73cd00 Compare August 30, 2017 17:00
@dmitrizagidulin
dmitrizagidulin merged commit 49fb741 into dz_oidc Aug 30, 2017
@dmitrizagidulin
dmitrizagidulin deleted the external-webids-username-password branch August 30, 2017 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants