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

operator token login_hint format #218

Merged
merged 4 commits into from
Nov 6, 2024
Merged

Conversation

AxelNennker
Copy link
Collaborator

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

It was proposed to login_hint to convey operator token to the Camara Authorization Server.
This way OIDF and IETF standards don't need to be extended.

Which issue(s) this PR fixes:

Fixes #145

@garciasolero
Copy link
Contributor

In my opinion, using acronyms is not a good idea because it makes it harder to recognize the meaning of the prefix at a glance. Additionally, if we add new prefixes in the future, although unlikely, the initials could potentially collide.

My proposal is to use login_hint=operator_token:...

@sebdewet
Copy link
Collaborator

In my opinion, using acronyms is not a good idea because it makes it harder to recognize the meaning of the prefix at a glance. Additionally, if we add new prefixes in the future, although unlikely, the initials could potentially collide.

My proposal is to use login_hint=operator_token:...

ipport is also an acronym ...
although I agree that it will be clearer to use operatortoken as specified in TS43

@eric-murray
Copy link
Collaborator

My view is that we should name login_hint prefixes as if they were URI schemes, mainly because they look like URI schemes (and, indeed, tel is a URI scheme). The rules on URI scheme naming are:

scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )

This would exclude underscores from valid names.

Note that URI schemes are case-insensitive. So TEL: would be as acceptable as tel:. If this is not intended, it should probably be clarified.

@AxelNennker
Copy link
Collaborator Author

Note that URI schemes are case-insensitive. So TEL: would be as acceptable as tel:. If this is not intended, it should probably be clarified.

This document defines prefixes to be used in login_hint. If this document specifies tel: then the prefix is tel:. This document does not state the the prefixes are uri schemes.

If you insist, we can add text but I do not see a need.

@AxelNennker
Copy link
Collaborator Author

In my opinion, using acronyms is not a good idea because it makes it harder to recognize the meaning of the prefix at a glance. Additionally, if we add new prefixes in the future, although unlikely, the initials could potentially collide.

My proposal is to use login_hint=operator_token:...

@garciasolero Are you OK with @sebdewet 's proposal to use operatortoken: ?

@eric-murray
Copy link
Collaborator

eric-murray commented Nov 5, 2024

@AxelNennker

This document does not state the the prefixes are uri schemes.

Actually, it does:

For phone numbers. The login_hint must be a tel URI as defined in RFC 3966 for global phone numbers without visual separators in E.164 format. For example, tel:+34666666666.

RFC 3966 defines tel: as a URI scheme. And it also states "All parameter names and values SHOULD use lower-case characters", but does not mandate it. So TEL: is allowed, albeit discouraged.

@AxelNennker
Copy link
Collaborator Author

My view is that we should name login_hint prefixes as if they were URI schemes, mainly because they look like URI schemes (and, indeed, tel is a URI scheme). The rules on URI scheme naming are:

scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )

This would exclude underscores from valid names.

Note that URI schemes are case-insensitive. So TEL: would be as acceptable as tel:. If this is not intended, it should probably be clarified.

I think URI schemes introduce more problems than bring benefit. I would keep the prefixes as defined.
@eric-murray is that OK for you?

@sebdewet please review.

@eric-murray
Copy link
Collaborator

is that OK for you?

Yes, operatortoken is fine for me because that does indeed comply with URI scheme naming rules

Copy link
Collaborator

@sebdewet sebdewet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok if we keep tel: ipport: and operatortoken:

@AxelNennker AxelNennker merged commit d7e1940 into main Nov 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to use operator_token (from TS43) to identify device on authentication request
5 participants