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

The need_info response doesn't say that the ticket is REQUIRED #275

Closed
xmlgrrl opened this issue Jan 22, 2017 · 6 comments
Closed

The need_info response doesn't say that the ticket is REQUIRED #275

xmlgrrl opened this issue Jan 22, 2017 · 6 comments
Labels
core Related to (original UMA1) core spec scope; may use obsolete language V2.0

Comments

@xmlgrrl
Copy link

xmlgrrl commented Jan 22, 2017

The ticket part of need_info is underspecified in this respect, and in fact, there's no ticket value included in the "full" example of a need_info response with error_details hints (Core rev 12 Sec 3.6.7). This needs to be fixed.

@xmlgrrl xmlgrrl added core Related to (original UMA1) core spec scope; may use obsolete language editorial Related to non-normative text errors V2.0 labels Jan 22, 2017
@xmlgrrl xmlgrrl removed the editorial Related to non-normative text errors label Feb 6, 2017
@xmlgrrl
Copy link
Author

xmlgrrl commented Feb 6, 2017

Actually, this probably needs a quick bit of discussion; I took off the editorial label. Now that we've required the ticket to rotate, the original squishy wording (connected to redirect_user) that we've updated seems to suggest that we should promote the ticket property out of the optional error_details structure entirely.

Old (as of Core 2.0 rev 01 Sec 3.5.4.2): "The permission ticket that was in the client's request for authorization data. If the authorization server provides the redirect_user property, it MAY also provide the ticket property. If it is provided, the client SHOULD NOT depend on the ticket's accuracy. Note: The appearance of the permission ticket is deprecated and will be removed in a future UMA version. It is included here for backwards compatibility."

Latest (as of Core 2.0 rev 13 Sec 3.6.8): "A permission ticket that allows the client to make further requests to the authorization server during this attempted authorization. The value of this permission ticket MUST NOT be the same as the one the client used to make its request."

We can say it's REQUIRED, but the problem is that the entire error_details structure is OPTIONAL, and need_info really needs to provide a permission ticket because the client is counting on the ticket to continue the authorization process. So should the internal structure be REQUIRED ticket + OPTIONAL error_details?

@xmlgrrl
Copy link
Author

xmlgrrl commented Feb 21, 2017

In ad hoc telecon 2017-02-21, we discussed, and agree. The specifics are laid out in an email to the WG. The underlying issue stems from our V1-era non-rotating ticket paradigm.

@xmlgrrl
Copy link
Author

xmlgrrl commented Feb 23, 2017

Here's what the email said:

There are three problems.

  • The structural problem is that the error_details structure is always optional, but that's where ticket lives. This could be fixed by moving ticket just outside of error_details.
  • The spec text problem is that it's underspecified when ticket is supposed to appear. It MUST appear when the AS responds with need_info, and it MUST NOT appear otherwise.
  • The example problem is that we've got a need_info example that doesn't show a ticket in it. That's easily fixed once we're certain how to fix the other two.

@xmlgrrl
Copy link
Author

xmlgrrl commented Feb 23, 2017

Closed in Core 2.0 rev 16.

@xmlgrrl xmlgrrl closed this as completed Feb 23, 2017
@jricher
Copy link

jricher commented Feb 23, 2017

Part of the question on the call was if we were allowed to put ticket higher up in the error structure, because 6749 doesn't specify an extension mechanism. I'd like to point out here that error_details is already an extension to the error structure as it's not specified in 6749, so we're already doing what we weren't sure we could do.

@xmlgrrl
Copy link
Author

xmlgrrl commented Feb 23, 2017

Ah, yes. That means I should probably add a bit more description to the new Core rev 16 Sec 9.4.2, to mention error_details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to (original UMA1) core spec scope; may use obsolete language V2.0
Projects
None yet
Development

No branches or pull requests

2 participants