Various Documentation improvements #27

jaxoncreed · 2019-05-14T19:13:47Z

Covers:

Describes the use case and implementation of PoP tokens
The description of asking for consent on the example page is more in line with reality
Extremely detailed walkthrough

README.md

michielbdejong

in the png, '6. Generates and id_token' -> '6. Generates an id_token'
step 5 of 'sending request' seems incorrect to me, IIUC RP only contacts OP to retrieve their jwks, not to ask them to perform validation.

zenomt · 2019-05-23T17:19:23Z

is this PR just to document how the reference implementation currently works, or to also suggest new functionality that is yet to be implemented? i see there's mention (in "README.md -> The Solution, step 3"), of the origin of the redirect_uri becoming "the audience".

i haven't checked the source to see if that's what the current NSS OP actually does, but regardless that has some issues (most importantly, the aud MUST include the client_id, which as discussed in #12 the client_id can't be just the redirect_uri origin, or even the redirect_uri). i think at least that part should be called out as "as currently implemented" or "proposed behavior" so that it can be commented on appropriately.

if this is documenting what's currently implemented, or at least what's currently designed/planned, then discussing issues with it isn't appropriate in this PR's conversation, and should be in an actual Issue once this change is merged. however, if this PR is also proposing new behavior, then discussing issues with the new behavior would be appropriate here.

michielbdejong · 2019-06-13T09:10:19Z

@jaxoncreed ping

jaxoncreed · 2019-06-14T02:45:13Z

I've now finished the detailed application workflow, so I think we're good to go. It's not the 100% rewrite this needs, but it provides some much better context.

michielbdejong

I didn't read/verify all of it but looks like a great improvement over the current text. We can always iterate if we discover errata. A second review from @zenomt and/or @dmitrizagidulin would be helpful!

michielbdejong · 2019-06-14T07:04:51Z

We should probably update the list of people with write access to this repo to match the list of people who know most about how webid-oidc works. Created #32 in relation to this.

michielbdejong · 2019-06-14T09:32:00Z

@kjetilk @Mitzi-Laszlo @timbl @justinwb @RubenVerborgh: two of you need to approve this before it can be merged.

RubenVerborgh · 2019-06-14T10:25:38Z

@michielbdejong Will do; useful to use the "assign reviewers" functionality for that.

RubenVerborgh

Technically correct, but I feel that we are too much describing an existing spec here. We should point more and explain less (unless the actual specs are not clear). However, good from a documentation perspective, but need to revisited when turning into an actual spec, where we want to avoid repetition of other specs.

RubenVerborgh · 2019-06-14T10:26:49Z

README.md

@@ -43,7 +43,7 @@ See also: [Motivation for WebID-OIDC](motivation.md).
 ### Benefits and Capabilities

 * Fully decentralized cross-domain authentication (any peer node can serve as
-  an identity provider as well as a relying party to any other node)
+  an identity provider as well as a relying party to any other node) made possible by [PoP Tokens](https://tools.ietf.org/html/rfc7800).


Perhaps say a word about what they are, or at least expand acronym?

RubenVerborgh · 2019-06-14T10:27:34Z

README.md

+
+#### The Problem
+
+Unlike standard implementations of OIDC, WebID-OIDC must deal with a number of RSs many of which the OP will not know about. OIDC defines the `aud` claim which defines the RSs for which a token can be used.


But we are using a standard, right? Important that people understand this is not a custom extension.

application-workflow-detailed.md

zenomt · 2019-06-14T18:01:41Z

in the current version of the sequence diagram, authorization steps 3 and 4 (request OP configuration) are to Alice's POD, but i think you mean for them to go to her OP.

application-workflow-detailed.md

jaxoncreed · 2019-06-19T11:47:16Z

@zenomt right you are

application-workflow-detailed.md

TallTed · 2019-06-27T15:18:33Z

application-workflow-detailed.md

+
+That URL might look a little complex, but it's essentially a request to `https://secureauth.example/authorize` with the following URL parameters:
+
+ - `scope=open_id`: a list of [oidc scpes](https://auth0.com/docs/scopes/current/oidc-scopes) (attributes of the RS to which this token should have access). `open_id` is a scope that is needed to verify Alice's identity.


[oidc scpes]
-> [OIDC scopes]

TallTed · 2019-06-27T15:19:06Z

application-workflow-detailed.md

+ - `scope=open_id`: a list of [oidc scpes](https://auth0.com/docs/scopes/current/oidc-scopes) (attributes of the RS to which this token should have access). `open_id` is a scope that is needed to verify Alice's identity.
+ - `client_id=7243fd594bdcf9c71a9b902274afaa30`: indicates the id of the client. The value for this field should be obtained in the registration phase.
+ - `response_type=id_token%20token` indicates the desired response data. Note that you cannot use response types that were not previously indicated during registration.
+ - `request=eyJhbGciOiJub25lIn0.eyJyZWRpc...`: A JWT containing the public key of the client and signed by the client using the private key. This is unique to webId-oidc. We will eventually use this to generate our pop-token.


webId-oidc
-> WebID-OIDC

TallTed · 2019-06-28T18:55:19Z

application-workflow-detailed.md

 - `client_id=7243fd594bdcf9c71a9b902274afaa30`: indicates the id of the client. The value for this field should be obtained in the registration phase.
 - `response_type=id_token%20token` indicates the desired response data. Note that you cannot use response types that were not previously indicated during registration.
- - `request=eyJhbGciOiJub25lIn0.eyJyZWRpc...`: A JWT containing the public key of the client and signed by the client using the private key. This is unique to webId-oidc. We will eventually use this to generate our pop-token.
+ - `request=eyJhbGciOiJub25lIn0.eyJyZWRpc...`: A JWT containing the public key of the client and signed by the client using the private key. This is unique to WebId-OIDC. We will eventually use this to generate our pop-token.


hopefully clearer...
WebId
-> WebID

jaxoncreed · 2019-07-29T14:25:01Z

Can we resurrect this and approve it @kjetilk. I keep sending links to this document because people keep asking how the login works and can't find this.

kjetilk · 2019-07-30T13:03:56Z

Can we resurrect this and approve it @kjetilk. I keep sending links to this document because people keep asking how the login works and can't find this.

Right! But as of now, it has no approving reviews... As I said above, I'd prefer that others provide reviews and then I review and merge it, because I don't feel up to the task of performing a technical review myself. So, @justinwb , @RubenVerborgh , @TallTed , @michielbdejong , please have another look and update your reviews.

RubenVerborgh · 2019-07-30T13:05:00Z

I can't; not knowledgeable enough about OIDC.

kjetilk · 2019-07-30T14:22:40Z

My problem too... I think the Authentication Panel should convene urgently to resolve this.

dmitrizagidulin · 2019-07-30T17:15:12Z

@kjetilk

I think the Authentication Panel should convene urgently to resolve this.

I agree!

dmitrizagidulin

Read through this carefully on a call with @jaxoncreed.

Looks great, thank you SO much for doing this work, Jackson!

TallTed · 2019-08-05T15:08:14Z

application-workflow-detailed.md

@@ -331,12 +331,12 @@ GET https://secureauth.example/authorize?scope=openid&client_id=7243fd594bdcf9c7

 That URL might look a little complex, but it's essentially a request to `https://secureauth.example/authorize` with the following URL parameters:

- - `scope=open_id`: a list of [OIDC scpes](https://auth0.com/docs/scopes/current/oidc-scopes) (attributes of the RS to which this token should have access). `open_id` is a scope that is needed to verify Alice's identity.
+ - `scope=openid`: a list of [OIDC scopes](https://auth0.com/docs/scopes/current/oidc-scopes) (attributes of the RS to which this token should have access). `open_id` is a scope that is needed to verify Alice's identity.


`scope=open_id`

became

`scope=openid`

so

`open_id` is a scope

should become

`openid` is a scope

justinwb · 2019-08-05T16:55:34Z

Should link to the new document at application-workflow-detailed.md, but the links to this and application-workflow.md should be put in context based on the use case. File names may also need to be changed to align with that and prevent confusion.

justinwb

Nice improvement to the current state of the documentation. There's still content required to provide full coverage, especially as it relates to the different patterns of use and how to apply them, but I think that this moves the ball forward in this current instantiation of the spec. The rest of the efforts should be channeled towards https://github.com/solid/specification. With that in mind, I'm good with approving these changes here.

as [previously flagged](solid#27 (comment))

PoP tokens documentation

a30c4c5

michielbdejong reviewed May 14, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

michielbdejong reviewed May 14, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

jaxoncreed commented May 14, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

jaxoncreed added 4 commits May 14, 2019 19:10

Fix dmitri and michiel issues

c64b85b

Replaced wrongly identified RP with RS

c10527b

In depth documentation up to auth request

cf229ee

Update sequence Diagram

7bff76a

michielbdejong reviewed May 16, 2019

View reviewed changes

Finish the detailed application workflow

6e50af8

jaxoncreed changed the title ~~[WIP] Various Documentation improvements~~ Various Documentation improvements Jun 14, 2019

Change photo filename

0f24e25

michielbdejong previously approved these changes Jun 14, 2019

View reviewed changes

RubenVerborgh requested review from timbl, kjetilk, justinwb and RubenVerborgh June 14, 2019 10:25

RubenVerborgh previously approved these changes Jun 14, 2019

View reviewed changes

Ruben fixes

0fa853b

jaxoncreed dismissed stale reviews from RubenVerborgh and michielbdejong via 0fa853b June 14, 2019 15:47

justinwb reviewed Jun 14, 2019

View reviewed changes

application-workflow-detailed.md Outdated Show resolved Hide resolved

justinwb reviewed Jun 14, 2019

View reviewed changes

application-workflow-detailed.md Outdated Show resolved Hide resolved

TallTed reviewed Jun 19, 2019

View reviewed changes

application-workflow-detailed.md Outdated Show resolved Hide resolved

Updated numbers

d389c3e

zenomt mentioned this pull request Jun 25, 2019

How do we identify apps? #33

Open

Fixed spellin/grammar

9237fdc

TallTed reviewed Jun 27, 2019

View reviewed changes

Capitalize OIDC

4990732

TallTed reviewed Jun 28, 2019

View reviewed changes

Fix WebID Capitalization

ece65c9

michielbdejong mentioned this pull request Jul 1, 2019

Version 0.8 Changelog solid/solid-spec#199

Closed

Small corrections

e1725a1

dmitrizagidulin previously approved these changes Aug 5, 2019

View reviewed changes

TallTed reviewed Aug 5, 2019

View reviewed changes

justinwb self-requested a review August 5, 2019 16:43

Added link to Application Workflow Detailed

a54c0b4

jaxoncreed dismissed dmitrizagidulin’s stale review via a54c0b4 August 5, 2019 17:07

Changed file name to avoid confusion

ed1aaaf

justinwb approved these changes Aug 6, 2019

View reviewed changes

dmitrizagidulin approved these changes Aug 6, 2019

View reviewed changes

zenomt mentioned this pull request Aug 11, 2019

substantial and significant security issues with current "bearer token" scheme solid/authentication-panel#1

Closed

dmitrizagidulin merged commit 0e6da67 into solid:master Aug 12, 2019

TallTed added a commit to TallTed/webid-oidc-spec that referenced this pull request Aug 12, 2019

change explanatory text to match code it explains

4150131

as [previously flagged](solid#27 (comment))

TallTed mentioned this pull request Aug 12, 2019

change explanatory text to match code it explains #36

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various Documentation improvements #27

Various Documentation improvements #27

jaxoncreed commented May 14, 2019 •

edited

Loading

michielbdejong left a comment

zenomt commented May 23, 2019

michielbdejong commented Jun 13, 2019

jaxoncreed commented Jun 14, 2019

michielbdejong left a comment

michielbdejong commented Jun 14, 2019

michielbdejong commented Jun 14, 2019

RubenVerborgh commented Jun 14, 2019

RubenVerborgh left a comment

RubenVerborgh Jun 14, 2019

RubenVerborgh Jun 14, 2019

zenomt commented Jun 14, 2019

jaxoncreed commented Jun 19, 2019

TallTed Jun 27, 2019

TallTed Jun 27, 2019

TallTed Jun 28, 2019

jaxoncreed commented Jul 29, 2019

kjetilk commented Jul 30, 2019

RubenVerborgh commented Jul 30, 2019

kjetilk commented Jul 30, 2019

dmitrizagidulin commented Jul 30, 2019

dmitrizagidulin left a comment

TallTed Aug 5, 2019

justinwb commented Aug 5, 2019

justinwb left a comment


		#### The Problem

		Unlike standard implementations of OIDC, WebID-OIDC must deal with a number of RSs many of which the OP will not know about. OIDC defines the `aud` claim which defines the RSs for which a token can be used.


		That URL might look a little complex, but it's essentially a request to `https://secureauth.example/authorize` with the following URL parameters:

		- `scope=open_id`: a list of [oidc scpes](https://auth0.com/docs/scopes/current/oidc-scopes) (attributes of the RS to which this token should have access). `open_id` is a scope that is needed to verify Alice's identity.

Various Documentation improvements #27

Various Documentation improvements #27

Conversation

jaxoncreed commented May 14, 2019 • edited Loading

michielbdejong left a comment

Choose a reason for hiding this comment

zenomt commented May 23, 2019

michielbdejong commented Jun 13, 2019

jaxoncreed commented Jun 14, 2019

michielbdejong left a comment

Choose a reason for hiding this comment

michielbdejong commented Jun 14, 2019

michielbdejong commented Jun 14, 2019

RubenVerborgh commented Jun 14, 2019

RubenVerborgh left a comment

Choose a reason for hiding this comment

RubenVerborgh Jun 14, 2019

Choose a reason for hiding this comment

RubenVerborgh Jun 14, 2019

Choose a reason for hiding this comment

zenomt commented Jun 14, 2019

jaxoncreed commented Jun 19, 2019

TallTed Jun 27, 2019

Choose a reason for hiding this comment

TallTed Jun 27, 2019

Choose a reason for hiding this comment

TallTed Jun 28, 2019

Choose a reason for hiding this comment

jaxoncreed commented Jul 29, 2019

kjetilk commented Jul 30, 2019

RubenVerborgh commented Jul 30, 2019

kjetilk commented Jul 30, 2019

dmitrizagidulin commented Jul 30, 2019

dmitrizagidulin left a comment

Choose a reason for hiding this comment

TallTed Aug 5, 2019

Choose a reason for hiding this comment

justinwb commented Aug 5, 2019

justinwb left a comment

Choose a reason for hiding this comment

jaxoncreed commented May 14, 2019 •

edited

Loading