nodeSolidServer / solid-auth-client

A browser library for performing authenticated requests to Solid pods
https://solid.github.io/solid-auth-client/
MIT License
95 stars 42 forks source link

solid:oidcIssuer discovery #107

Open elf-pavlik opened 5 years ago

elf-pavlik commented 5 years ago

https://github.com/solid/webid-oidc-spec#issuer-discovery-from-webid-profile

During the Provider Selection or Provider Confirmation steps it is necessary to discover, for a given WebID, the URI of the authorized OIDC provider for that WebID.

I have currently initial deployment of NSS5.0.0-beta.6 on one domain to act as Resource Server holding my WebID Profile. I also have another deployment of NSS5.0.0-beta.6 to act as Identity Provider (OIDC Issuer). When I test latest master of this repo and enter my WebID in the popup, it makes request directly to /authorize of server hosting my WebID and doesn't try to discover solid:oidcIssuer from my WebID profile.

dmitrizagidulin commented 5 years ago

Ah, interesting. I think I know why that is (has to do with design assumptions). I’ll take a look at the code, to be sure.

elf-pavlik commented 5 years ago

It looks that indeed `webid-oidc.js' makes some assumptions it shouldn't.

const registration = {
    issuer: idp,
    grant_types: ['implicit'],
    redirect_uris: [callbackUri],
    response_types: [responseType],
    scope: 'openid profile'
}

While IdpSelect.js has

<p>Please enter your WebID or the URL of your identity provider:</p>

And handleSelectIdp() just passes the WebID calling

await auth.login(idp, loginOptions)

What do you think if we had login(webIdOrIdp) and first step would check if de-referenced returns WebID Profile and in that case do discovery based on solid:oidcIssuer ?

elf-pavlik commented 5 years ago

I started playing with simple implementation, which adds a function to webid-oidc.js to discover OIDC Issuer from WebID Profile and gets used as first step of WebIdOidc.login(). It needs to parse turtle, currently I use https://github.com/rdfjs/parser-n3 but preferably it should use dependency injection so that application can provide a parser which it already imports. Do you think it should get injected using SolidAuthClient constructor?

RubenVerborgh commented 5 years ago

We have such a function somewhere, possibly solid auth manager.

elf-pavlik commented 5 years ago

I guess this one: https://github.com/solid/oidc-auth-manager/blob/master/src/preferred-provider.js#L22 It also seems to make incorrect assumptions. WebID Profile can stay hosted on server also hosting OIDC provider but in the WebID Profile one can specify other issuer. I think it has to check the WebID Profile first and if it doesn't include solid:oidcIssuer statement then fall back to /.well-known based discovery.

I also see it imports rdflib while I only needed https://github.com/rdfjs/parser-n3 and checked for statement with solid:oidcIssuer directly on data event during parsing.

RubenVerborgh commented 5 years ago

True, I think we can do better indeed 🙂 Constructor param maybe a little too far for now; overhead should be minimal.

dmitrizagidulin commented 5 years ago

It also seems to make incorrect assumptions. WebID Profile can stay hosted on server also hosting OIDC provider but in the WebID Profile one can specify other issuer. I think it has to check the WebID Profile first

So just for background - that assumption was basically a performance optimization. The WebID profile only gets checked for an external oidcIssuer only if it's different from the serverUri, not on every request, since I think that might be excessive.

elf-pavlik commented 5 years ago

I've created issues directly on oidc-auth-manager

elf-pavlik commented 5 years ago

This commit has my simple working experiment that discovers solid:oidcIssuer from WebID Profile: https://github.com/elf-pavlik/solid-auth-client/commit/57007a9cbb93cf77389e6c9c7b39929417cec688#diff-2386e1ce48920481f393939fd08834d1

I could extract https://github.com/solid/oidc-auth-manager/blob/master/src/preferred-provider.js into separate module and use it instead. At the same time replacing rdflib with just turtle parser.

Looking at http://chrisbateman.github.io/webpack-visualizer/ on my experimental branch it still adds quite a bit by bundling N3.js and readable-stream. Dependency injection with the parser and using something lighter than readable-stream would mostly address that bundle size increase.

elf-pavlik commented 5 years ago

I don't see a clean way of injecting turtle parser dependency. This module exports a singleton instance of SolidAuthClient. If it exported the class I could imagine application importing it to simply:

const auth = new SolidAuthClient({ turtleParser: new N3Parser() })

and later in SolidAuthClient#login it could get passed to WebIdOidc#login

elf-pavlik commented 5 years ago

If this module exports the class, binding methods to instance could happen in the constructor, possibly using https://github.com/sindresorhus/auto-bind

RubenVerborgh commented 5 years ago

webpack can just do it, no?

elf-pavlik commented 5 years ago

I don't understand how webpack can provide a clear solution. I would like to develop Solid based Progressive Web App. In that application I would like to import:

If the application has useful features without login, I will most likely use dynamic import() to lazy load solid-auth-client once user decides to login. On the other hand @rdfjs/parser-n3 will get loaded much earlier in the application shell. Once I import() solid-auth-client I imagine creating an instance of it this way:

import ParserN3 from '/node_modules/rdfjs/parser-n3/index.js'
// ...

// when user decides to login
const SolidAuthClient = await import('/node_modules/solid-auth-client/module/index.js')
const auth = new SolidAuthClient({ turtleParser: new ParserN3() })

This would work in a similar way as modules which depend on LevelDOWN, for example: https://www.npmjs.com/package/quadstore#rdfstore-class

import memdown from 'memdown'
import { RdfStore } from 'quadstore'
const store = new RdfStore(memdown())

or using different LevelDOWN implementation

import leveldown from 'leveldown'
import { RdfStore } from 'quadstore'
const store = new RdfStore(leveldown('.db'))
RubenVerborgh commented 5 years ago

I don't understand how webpack can provide a clear solution.

So webpack could either a) bundle a different dependency b) leave the dependency open, for the scenario you describe above.

But indeed, your scenario with multi-stage loading (which I like BTW) is more complex. I like the idea of the window.solid namespace, which currently has solid.auth and solid.data. If something does not ship with an auth implementation, it just uses solid.auth (thanks to webpack); we could do the same with solid.rdfjs for instance.