Skip to content

feat: allow openId to choose an unactive email if there are none active#6041

Merged
rjsparks merged 3 commits intoietf-tools:mainfrom
rjsparks:openidemail
Jul 25, 2023
Merged

feat: allow openId to choose an unactive email if there are none active#6041
rjsparks merged 3 commits intoietf-tools:mainfrom
rjsparks:openidemail

Conversation

@rjsparks
Copy link
Copy Markdown
Member

No description provided.

@rjsparks
Copy link
Copy Markdown
Member Author

cc: @rpcross @jennifer-richards

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 24, 2023

Codecov Report

Merging #6041 (9c0df2d) into main (09f3477) will decrease coverage by 0.01%.
The diff coverage is 91.11%.

@@            Coverage Diff             @@
##             main    #6041      +/-   ##
==========================================
- Coverage   88.67%   88.66%   -0.01%     
==========================================
  Files         288      290       +2     
  Lines       40001    40195     +194     
==========================================
+ Hits        35471    35640     +169     
- Misses       4530     4555      +25     
Files Changed Coverage Δ
ietf/doc/views_help.py 65.51% <ø> (ø)
ietf/doc/views_stats.py 74.80% <ø> (ø)
ietf/group/urls.py 100.00% <ø> (ø)
ietf/ietfauth/widgets.py 84.61% <ø> (ø)
ietf/submit/views.py 84.60% <ø> (ø)
ietf/group/views.py 90.25% <37.50%> (-0.29%) ⬇️
ietf/nomcom/views.py 92.90% <50.00%> (-0.22%) ⬇️
ietf/doc/views_doc.py 91.26% <79.16%> (-0.36%) ⬇️
ietf/person/models.py 91.83% <87.50%> (-0.12%) ⬇️
ietf/doc/views_statement.py 94.96% <94.96%> (ø)
... and 18 more

... and 1 file with indirect coverage changes

@jennifer-richards
Copy link
Copy Markdown
Member

I am not familiar with what is done with the openid claims, but is it a problem to be handing back an email address that someone inactivated? If the other end of the exchange can be changed to handle the empty claim I'd think that's preferable to propagating an address that was deactivated. If not, maybe better to transmit a canned and identifiable "not-an-address"?

Also, why "unactive" rather than "inactive"?

@rpcross
Copy link
Copy Markdown
Collaborator

rpcross commented Jul 25, 2023

We currently use claims.email from Datatracker as the email address and unique (DT related) identifier of the registration, used when calling DT APIs. An empty or generic string would break this.

@rjsparks
Copy link
Copy Markdown
Member Author

Handing the inactive address is going to do less harm than returning an empty string. There's no promise to openId that that email address still works - it's just the best email address we have from someone.
I'll change unactive to inactive.

@jennifer-richards
Copy link
Copy Markdown
Member

Thanks for the explanations.

Copy link
Copy Markdown
Member

@jennifer-richards jennifer-richards left a comment

Choose a reason for hiding this comment

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

Assuming you'll change the "u" to "i" in the names and approving.

@rjsparks rjsparks merged commit 593bdb4 into ietf-tools:main Jul 25, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 29, 2023
@rjsparks rjsparks deleted the openidemail branch August 10, 2023 22:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants