Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.2.1 JWT Bearer Tokens - "sub" claim #7

Closed
jaronaz opened this issue Nov 19, 2019 · 3 comments · Fixed by #21
Closed

3.2.1 JWT Bearer Tokens - "sub" claim #7

jaronaz opened this issue Nov 19, 2019 · 3 comments · Fixed by #21
Labels
Actie Logius WG Overleg: TO-OAuth Te agenderen voor het Technisch Overleg OAuth-NL Status: Klaar voor release Het voorstel is verwerkt en klaar voor de volgende release to be discussed in WG Type: Correctie Correctie van een (kleine) fout.

Comments

@jaronaz
Copy link

jaronaz commented Nov 19, 2019

In paragraaf 3.2.1 staat m.b.t. de opbouw/inhoud van het geretourneerde Bearer Token (JWT) naar de client het volgende:

The server MAY issue tokens with additional fields, including the following as defined here:

sub
The identifier of the end-user that authorized this client, or the client id of a client acting on its own behalf (such as a bulk transfer). Since this information could potentially leak private user information, it should be used only when needed. End-user identifiers SHOULD be pairwise anonymous identifiers unless the audiance requires otherwise.

iGov-NL
In iGov-NL the sub claim MUST be present.
/iGov-NL

Voor onze implementatie geldt:

  • end-user that authorized this client = burger (bsn mag in geen geval meegegeven worden)
  • client id of a client acting on its own behalf = n.v.t.

Vragen:

  • Waarom is hier gekozen voor MUST? Voor de implementatie die we nu doen met dit koppelvlak, komt het ons beter uit om dit veld niet op te nemen.
  • Waarom staat er dan toch "Since this information could potentially leak private user information, it should be used only when needed." Moet dit niet doorgehaald worden?
  • Mag deze waarde ook leeg zijn?
  • Het voorbeeld in deze paragraaf mist dit veld?
@mrtn78
Copy link
Member

mrtn78 commented Dec 9, 2021

op basis van alleen de tekst is mij dit ook onduidelijk. wellicht kan iemand zich herinneren wat de intentie was en dit beter verwoorden? Dan kunnen we de nieuwe beschrijving bespreken in de volgende werkgroep bijeenkomst.

@jaronaz
Copy link
Author

jaronaz commented Dec 13, 2021

Ik kan wel vertellen hoe we hier uiteindelijk mee zijn omgegaan:
Bij de implementatie van dit OAuth profiel hebben we destijds uiteindelijk gekozen om het sub te vullen met een transient-id, welke we aan de server kant loggen en de relatie leggen met het (static) id van de user (in dit geval BSN).
Op die manier blijft het mogelijk (met hulp van de uitgever) om als ontvanger van het token de relatie te leggen met de user (wanneer dit niet 1-op-1 doorgegeven mag worden).

Ik denk dat het dus wel goed is om dit als MUST op te nemen, omdat je daarmee ook mogelijk maakt dat de resource server zelfstandig de user resource kan ophalen. Andersom, als je dit veld weg zou mogen laten, dan moet je op een andere manier de link met de juiste resource leggen, dat is waarschijnlijk ongewenst.

rschaar-logius added a commit that referenced this issue Dec 28, 2021
Text suggestion for resolving issue #7.

The inconsistency w.r.t. the sub claim results from extending the iGov profile. The iGov profile supports different use cases, which makes an optional sub claim logical. The NL-gov profile currently only supports a use case for which the presence of a sub claim will be required. The extension was too minimal, leading to an inconsistency. This proposed change updates the text on the sub claim, resolving the inconsistency.
@rschaar-logius
Copy link
Collaborator

Zie bovenstaande pull request voor een gesuggereerde wijzigin van de tekst om de inconsistentie te verhelpen. Gebruik zonder een 'sub' claim zou buiten descope van de use case ondersteund in dit profiel vallen. De inconsistentie komt voort uit het (te) minimalistisch wijzigen t.o.v. het iGov profiel.

@sanderke sanderke linked a pull request Feb 2, 2022 that will close this issue
@mrtn78 mrtn78 added Overleg: TO-DK Te agenderen voor het Technisch Overleg Digikoppeling Status: Klaar voor release Het voorstel is verwerkt en klaar voor de volgende release Type: Correctie Correctie van een (kleine) fout. labels Jun 29, 2022
@sanderke sanderke added Overleg: TO-OAuth Te agenderen voor het Technisch Overleg OAuth-NL and removed Overleg: TO-DK Te agenderen voor het Technisch Overleg Digikoppeling labels Jul 5, 2022
@mrtn78 mrtn78 added this to the v1.1 - Client credentials Flow milestone Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Actie Logius WG Overleg: TO-OAuth Te agenderen voor het Technisch Overleg OAuth-NL Status: Klaar voor release Het voorstel is verwerkt en klaar voor de volgende release to be discussed in WG Type: Correctie Correctie van een (kleine) fout.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants