Add support of multi oidcIssuer relations per one user's profile #30
Conversation
Apply changes from https://github.com/solid/oidc-auth-manager
Merge last changes from solid/oidc-auth-manager
Sync with solid/oidc-auth-manager
|
What's happening with this PR? /cc @justinwb |
There was a problem hiding this comment.
Agree with the added functionality but disagree with the implementation.
The "discover all" functionality is not what we need. We simply want to validate whether a given party is allowed to act as an issuer for a given user. So rather than a discovery method, we need a validation method. At no point do we need a list of all possible providers.
| // drop the path (provider origin only) | ||
| if (providerUri) { | ||
| providerUri = (new URL(providerUri)).origin | ||
| if (Array.isArray(providerUri)) { |
There was a problem hiding this comment.
This is not good; should always be an array.
There was a problem hiding this comment.
Could force it into an array with [].concat(providerUri) (if there are possible cases where it's not - which I guess shouldn't be in this case; nevertheless, it's a nice "hack" to know of)
| @@ -61,20 +61,48 @@ function providerExists (uri) { | |||
| * given Web ID, extracted from Link rel header or profile body. If no | |||
| * provider URI was found, reject with an error. | |||
| */ | |||
| function discoverProviderFor (webId) { | |||
| function discoverProviderFor (webId, issuer) { | |||
There was a problem hiding this comment.
So the method functionality has changed from discovering to validating? We probably need a method like isValidProvider then.
There was a problem hiding this comment.
Ok, I will update the code.
I tried to create fix with a minimal changes in the code.
| } else { | ||
| validateProviderUri(null, webId) // Throw an error if empty or invalid | ||
| } | ||
| } else { |
There was a problem hiding this comment.
The above logic is very messy.
| @@ -94,16 +122,30 @@ function discoverFromHeaders (webId) { | |||
| }) | |||
| } | |||
|
|
|||
| function discoverFromProfile (webId) { | |||
| function discoverAllFromProfile (webId) { | |||
There was a problem hiding this comment.
I think this should then also become validateFromProfile and accept a second issuer parameter. Will eliminate most of the messy logic above.
No description provided.