Show user OpenID URIs in their profile #1314

strk · 2017-03-17T21:43:10Z

So you can tell from a quick look if the users are really who you think
they are (if you know their openid)

lunny · 2017-03-18T03:05:44Z

maybe you could send a screenshot?

strk · 2017-03-18T08:07:39Z

@lunny:

ptman · 2017-03-18T10:49:32Z

looks good

lunny · 2017-03-18T10:54:28Z

text align ...

strk · 2017-03-18T14:02:56Z

What about text align @lunny ? Looks aligned here

lunny · 2017-03-18T15:35:02Z

@strk
s
h
j
0

strk · 2017-03-18T16:12:19Z

@lunny I don't understand your message

Fastidious · 2017-03-18T17:18:19Z

Why is it bold? What happens when the URI is https://this.is.my.openid.provider.of.choice.which.i.use.here/bananas? If the OpenID is to authenticate/login, why is there a need to show it? What is the benefit of it?

strk · 2017-03-18T19:46:20Z

Your OpenID basically shows who you are, so it's part of your identity. With OpenID that identity is "verified" (by Gitea, in this case). So you're saying the world you are "my.banan.as", and by showing it Gitea is telling the visitor that it verified you are. Good question about bold, doesn't need be. Changed with 9c7ccc2 What happens when URL is long ? I guess the display is bad, want to test it ? I can setup a staging install with it.

strk · 2017-03-18T19:56:13Z

Made a test with long OpenID (and no-more-bold). Here's what you get:

Maybe there's some client-side trick to adapt to screen in javascript ? @geek1011

Fastidious · 2017-03-18T20:02:22Z

Also, http://fontawesome.io/icon/openid/. The icon being used now is a blur of rubbish. 😄

strk · 2017-03-18T20:13:49Z

7ebf154 uses font-awesome, and here's a screenshot from another machine (with non-magnified browser content):

strk · 2017-03-18T20:18:21Z

And those links become clikable with f5b8cb6, and here's another screenshot:

strk · 2017-03-18T20:19:03Z

NOTE: from this other machine (Firefox) the overflow never happens (the first screenshot with overflow was from another browser, dunno now which one it was)

Fastidious · 2017-03-18T21:28:23Z

@strk you wrote:

Your OpenID basically shows who you are [...]

I though OpenID was about authentication, not identification. Am I wrong?

lunny · 2017-03-19T02:27:18Z

@strk maybe you should give an option to enable to show OpenID in the profile and default is disabled. Security is always the first class Gitea should consider.

strk · 2017-03-19T07:23:22Z

@Fastidious I think OpenID can serve both. By all means authentication is authentication of an identity (your URL). @lunny good point about allowing a flag to make visible/hidden, will work on it (maybe per-openid).
Speaking of which, I went looking how that's done for email and found a bug (#1323)

strk · 2017-03-19T14:05:03Z

Ready, there's now a per-OpenID show/hide toggle, defaulting to hide.

strk · 2017-03-19T14:07:42Z

pgaskin

LGTM except for my comments

pgaskin · 2017-03-19T17:25:59Z

options/locale/locale_en-US.ini

@@ -357,6 +357,8 @@ key_name = Key Name
 key_content = Content
 add_key_success = Your new SSH key '%s' has been added successfully!
 delete_key = Delete
+show_key = Show


It is kind of unclear what this does, maybe make it "Show on profile"

pgaskin · 2017-03-19T17:26:07Z

options/locale/locale_en-US.ini

@@ -357,6 +357,8 @@ key_name = Key Name
 key_content = Content
 add_key_success = Your new SSH key '%s' has been added successfully!
 delete_key = Delete
+show_key = Show
+hide_key = Hide


It is kind of unclear what this does, maybe make it "Hide on profile"

See how you like a117987

pgaskin · 2017-03-19T17:26:33Z

Make LGTM work

strk · 2017-03-19T17:56:36Z

Like this, @geek1011 ?

strk · 2017-03-19T20:08:09Z

I've also added unit test now.
Oh, and @Fastidious pointed out he thinks the text should be Hide in profile rather than Hide from profile, what do others think ?

lunny · 2017-03-20T01:49:29Z

I have no idea about Hide in profile or Hide from profile, after that, I will give it LGTM.

pgaskin · 2017-03-20T01:51:02Z

@lunny Hide in profile means you are putting it somewhere in the profile, and hide from profile means that you are not including it in the profile. I speak English as my first language.

bkcsoft · 2017-03-20T07:44:14Z

Conflicting migrations. Otherwise LGTM

bkcsoft · 2017-03-20T08:30:47Z

poof?

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 18, 2017

lunny added this to the 1.2.0 milestone Mar 18, 2017

lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Mar 18, 2017

strk force-pushed the showOpenID branch from c768516 to 7e64d71 Compare March 18, 2017 13:10

appleboy approved these changes Mar 18, 2017

View reviewed changes

strk force-pushed the showOpenID branch from c4f825c to f5b8cb6 Compare March 18, 2017 20:17

strk force-pushed the showOpenID branch from f5b8cb6 to e753947 Compare March 19, 2017 08:54

pgaskin approved these changes Mar 19, 2017

View reviewed changes

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 19, 2017

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 20, 2017

strk added 8 commits March 20, 2017 09:04

Show user OpenID URIs in their profile

a6f7c0b

Do not use bold for OpenID URI

754e64f

Use font-awesome openid logo in user profile

58b276f

Make user openid URIs clickable

4cb34e5

Add show/hide toggle for user's OpenID URIs (default to false)

92800d1

Rename openid show/hide keys and make strings more descriptive

fc223d9

Add unit tests for user_openid table

f3b4b6c

Use "Show on profile" wording

56b17e6

strk force-pushed the showOpenID branch from 504ed1e to 56b17e6 Compare March 20, 2017 08:07

bkcsoft merged commit 9182a35 into go-gitea:master Mar 20, 2017

strk deleted the showOpenID branch March 20, 2017 08:36

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Uh oh!

Show user OpenID URIs in their profile #1314

Show user OpenID URIs in their profile #1314

Uh oh!

Conversation

strk commented Mar 17, 2017

Uh oh!

lunny commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017

Uh oh!

ptman commented Mar 18, 2017

Uh oh!

lunny commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017 via email

Uh oh!

lunny commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017 via email

Uh oh!

Fastidious commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017 via email

Uh oh!

strk commented Mar 18, 2017

Uh oh!

Fastidious commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017

Uh oh!

strk commented Mar 18, 2017

Uh oh!

Fastidious commented Mar 18, 2017

Uh oh!

lunny commented Mar 19, 2017

Uh oh!

strk commented Mar 19, 2017

Uh oh!

strk commented Mar 19, 2017

Uh oh!

strk commented Mar 19, 2017

Uh oh!

pgaskin left a comment

Choose a reason for hiding this comment

Uh oh!

pgaskin Mar 19, 2017

Choose a reason for hiding this comment

Uh oh!

pgaskin Mar 19, 2017

Choose a reason for hiding this comment

Uh oh!

strk Mar 19, 2017

Choose a reason for hiding this comment

Uh oh!

pgaskin commented Mar 19, 2017

Uh oh!

strk commented Mar 19, 2017

Uh oh!

strk commented Mar 19, 2017

Uh oh!

lunny commented Mar 20, 2017

Uh oh!

pgaskin commented Mar 20, 2017

Uh oh!

bkcsoft commented Mar 20, 2017

Uh oh!

bkcsoft commented Mar 20, 2017

Uh oh!

Uh oh!