<div dir="ltr"><div>[most of this review is against a revision of the draft that I got from git yesterday and "built" from source but some changes have been committed while reviewing so some comments might be out of date but I've also tried to kinda look at the updates so my thoughts and comments might be more confused or confusing than normal] <br></div><div><br></div><div><u><br></u></div><div><u>===>1. Introduction </u><br></div><div>I sorta get the idea (I think) but also struggle to really follow it all. I know that's not actionable feedback and I apologize for that but it maybe gives me an opportunity to pivot into some more meta comments.<br></div><div>I brought up the <a href="https://tools.ietf.org/html/draft-ietf-oauth-incremental-authz-03">OAuth 2.0 Incremental Authorization</a> work on the Wednesday call not because I think it's better necessarily but there does seem to be a fair amount of overlap in what it does and what the goals appear to be. And it always raises questions/concerns in my mind to see what seem to be disparate spec solutions for the same problem. Incremental Authz did go through WGLC but progress on  it seems to have stalled. So it may be moot but supporting both standards at the same time would be super awkward (or worse). <br></div><div>There's also content in the introduction of the grant management draft (4th & 5th paragraphs) that appears to outright contradict Incremental Authz, which gives me pause. Incremental Authz also differentiates behaviour quite a bit between public and confidential clients for privacy and security reasons while there's no such distinction in the grant management draft. I haven't wrapped my head around it enough to say if it's a deficiency for grant management but it did stand out as a notable difference to me that likely needs more consideration and analysis. <br></div><div>The public vs confidential client thing gets me thinking too that most/all of FAPI has public clients out of scope or disallowed. And I think that, particularly if public clients were not in scope, something like the grant management API could be added on as a resource without changes to the OAuth authz endpoint and token endpoint flow. Maybe that wouldn't meet all the requirements behind this, I don't know, but it would definitely be a less invasive change and that usually means it'd be easier and thus more likely to get added into products and OSS projects. <br></div><div><br></div><div><u>===>2. Overview</u></div><div>I don't quite get the use of the word "disclosure" how it's used here alongside create and update in the first sentence. <br></div><div><br></div><div><u>===> 3.1. Authorization Request </u></div><div>I think the values and behaviours of the different grant_modes needs some more fleshing out. But looks like there've been some changes already recently here so I'm apparently not alone in that thinking. <br></div><div><br></div><div>I know it's only the example but could the value of the grant_id in the examples be something more concise than what looks to be a hex encoding of 256 bits. It offends my delicate sensibilities and looks to be the reason behind all the warnings from xml2rfc:</div><div><br></div><div><font size="1"><span style="font-family:monospace">fapi-grant-management-00.xml(87): Warning: Too long line found (L214), 10 characters longer than 72 characters: <br>        &grant_id=4d276a8ab980c436b4ffe0c1ff56c049b27e535b6f1266e734d9bca992509c25<br>fapi-grant-management-00.xml(116): Warning: Too long line found (L270), 11 characters longer than 72 characters: <br>      "grant_id":"4d276a8ab980c436b4ffe0c1ff56c049b27e535b6f1266e734d9bca992509c25"<br>fapi-grant-management-00.xml(167): Warning: Too long line found (L355), 25 characters longer than 72 characters: <br>   <a href="https://as.example.com/grants/4d276a8ab980c436b4ffe0c1ff56c049b27e535b6f1266e734d9bca992509c25">https://as.example.com/grants/4d276a8ab980c436b4ffe0c1ff56c049b27e535b6f1266e734d9bca992509c25</a><br>fapi-grant-management-00.xml(174): Warning: Too long line found (L362), 7 characters longer than 72 characters: <br>   GET /grants/4d276a8ab980c436b4ffe0c1ff56c049b27e535b6f1266e734d9bca992509c25<br>fapi-grant-management-00.xml(205): Warning: Too long line found (L408), 10 characters longer than 72 characters: <br>   DELETE /grants/4d276a8ab980c436b4ffe0c1ff56c049b27e535b6f1266e734d9bca992509c25<br> Created file fapi-grant-management-00.txt</span></font></div><div><br></div><div>Even base64url encoding the same underlying value would be a little shorter with TSdqirmAxDa0_-DB_1bASbJ- U1tvEmbnNNm8qZJQnCU but still more than seems needed (~<span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none">340,000,000,000,000,000,000,000,000,000,000,000,000 times as much entropy as the security tokens in the example which sends a strange message).  Subsets of that value like TSdqirmAxDa0_-DB_1bASbJ-U1tvEmbn or even TSdqirmAxDa0_-DB_1bASQ would make the warnings go away and still look to meet the uniqueness requirements.<br></span></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"><br></span></div><div><u><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none">===> 3.2.1. Error Response <br></span></u></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none">Not sure it necessarily belongs here as an error code but, with other text about a grant being bound to a user, I do think there needs to be more about the non-happy path cases of what happens when a grant_id is provided but the AS identifies a different user. It's probably just an error condition(?) but could use some more fleshing out me thinks. <br></span></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"><br></span></div><div><u><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none">===> 3.3. Token Request <br></span></u></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none">"The grant_mode and grant_id parameters MAY be added to the token request in case this request is also an authorization request." - after reading this several times I think I know what is meant but "authorization request" is kinda overloaded and results in this text being super confusing (to me anyway). It maybe better to just not support the grant_* parameters at the token endpoint. I struggle to see a real use case for them there. But maybe I'm not creative enough. Seems to just complicate things though. <br></span></div><div><br></div><div>Using the grant_mode and grant_id parameters at the CIBA auth endpoint is something to think about though... <br></div><div><br></div><div>===> 3.4. Token Response</div><div>I think Filip said something here already but the RECOMMENDED part here is pretty awkward. The "code verifier" is presumably a mistake? And the value in the example doesn't align with the recommendation.</div><div>Oh, wait, I see there's new text for that paragraph now "The <code>grant_id</code> value MUST be generated by the authorization 
server and constructed from a cryptographically strong random or 
pseudo-random number sequence (see [@RFC4086]] for best current 
practice) of at least 64 bits." - 64 bits is actually probably too few to reliably ensure uniqueness. And if bytes was intended there, 64 is kinda absurd for a MUST. <br></div><div>I think perhaps saying less here would be better. Like what's in the prior paragraph and similar in 3.1. is maybe sufficient.</div><div><br></div><div>"OPEN: Should the AS always return the grant_id if the client once asked for it?" - no</div><div><br></div><div>"OPEN: Should the AS rotate grant ids for privacy reasons?" - no <br></div><div><br></div><div><u> ===>  4.1. API authorization <br></u></div><div>Is the granularity of query and revoke really needed? Maybe just using one scope of grant_management would be okay? Sometimes less is more. <br></div><div><br></div><div>Are there other aspects of authorization that need to be called out here? Like it seems like the access token should also identify the same client as the grant_id. And maybe the user too? I guess that kinda depends on how one expects the access token for this will be acquired. <br></div><div><br></div><div><u>===>  4.2. Endpoint </u></div><div>Calling this a base URL/endpoint or something might be more clear about how this is used a bit different than other endpoints in metadata. Here this value is used with the grant_id to build the full endpoint, whereas most other endpoints are used as is and parameterized. <br></div><div><br></div><div><br></div><div><u>===>  4.5. Revoke Grant <br></u></div><div>I really don't want to make things more complicated than need be but I'm questioning the value of having just an all or nothing revoke function. Seems the same is possible via RFC 7009 revocation of a refresh token? <br></div><div>How would we expect a user to delete one part (a scope value or authz deets value) from a grant? That seems like something that would be wanted. Maybe with an authz request and update grant mode with the expectation that the AS allow for prior stuff to be un-selected? Or a replace with re-requesting everything and letting the user manage that way? I dunno, maybe I'm not seeing the bigger picture here or something. <br></div><div><br></div><div><br></div><div>Okay, that's all for a first review :) <br></div><div><u><br></u></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><u></u></div><div><br></div><div><br> </div><div><br></div><div><br></div><div><br></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"></span><u><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"></span></u></div><div><u><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"><br></span></u></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"><br></span></div><div><u><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"></span></u></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"><br></span></div><div><span style="color:rgb(68,68,68);font-family:Arial,Tahoma,Helvetica,FreeSans,sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;display:inline;float:none"><br></span></div><div><br></div><div><br></div></div>

<br>
<i style="margin:0px;padding:0px;border:0px;outline:0px;vertical-align:baseline;background:rgb(255,255,255);font-family:proxima-nova-zendesk,system-ui,-apple-system,system-ui,"Segoe UI",Roboto,Oxygen-Sans,Ubuntu,Cantarell,"Helvetica Neue",Arial,sans-serif;color:rgb(85,85,85)"><span style="margin:0px;padding:0px;border:0px;outline:0px;vertical-align:baseline;background:transparent;font-family:proxima-nova-zendesk,system-ui,-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,Oxygen-Sans,Ubuntu,Cantarell,"Helvetica Neue",Arial,sans-serif;font-weight:600"><font size="2">CONFIDENTIALITY NOTICE: This email may contain confidential and privileged material for the sole use of the intended recipient(s). Any review, use, distribution or disclosure by others is strictly prohibited.  If you have received this communication in error, please notify the sender immediately by e-mail and delete the message and any file attachments from your computer. Thank you.</font></span></i>