#11914 closed enhancement (fixed)
[Patch] OSM username in titlebar
Reported by: | Allroads | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 20.01 |
Component: | Core | Version: | |
Keywords: | Cc: | tpikonen@…, Klumbumbus, Polarbear-j, stoecker, Don-vip |
Description (last modified by )
Attachments (8)
Change History (46)
by , 10 years ago
Attachment: | JOSM Username.PNG added |
---|
comment:1 by , 10 years ago
Description: | modified (diff) |
---|
comment:3 by , 10 years ago
When implementing the profile button, one could also take #2710 into account.
comment:5 by , 8 years ago
Cc: | added |
---|
comment:7 by , 8 years ago
While creating a new ticket, I was checking for potential open one. I found it :-)
JOSM main windows - osm multi user account management / transparency
Add/Show current logged in OSM username in title bar (or status bar)
I can't estimate about time consumption for this implementation. But if it's easier to put the name of the active OSM user to title bar, I would appreciate please start with this enhancement.
The mentioned comfortable user switch function could be integrated later (independently).
Thanks a lot
comment:9 by , 8 years ago
Cc: | added |
---|
by , 5 years ago
Attachment: | 11914.patch added |
---|
It is easy to display the username but I don't know how to format it.
comment:10 by , 5 years ago
Summary: | OSM username in titlebar → [Patch] OSM username in titlebar |
---|
comment:11 by , 5 years ago
Maybe follow the Twitter & Co @
syntax, such as Java OpenStreetMap Editor (@GerdP)
?
comment:13 by , 5 years ago
I favour the @
variant, but this is just my personal taste.
Some remarks:
- We use
tr(" ({0})", string)
in a few other places to allow addressing different spacing styles around parenthesis - The title does not update after setting/changing the OSM user in the preferences. (Implementing this might be out of scope since
UserIdentityManager
cannot notify listeners at the moment.)
comment:14 by , 5 years ago
- Thanks for the hint, will use that
- Hmm, yes, the missing listener is not nice. I assumed that a mapper would have different configurations for his user names. In that case the user would not change within a session.
follow-up: 16 comment:15 by , 5 years ago
- Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).
- Remember that username may be non-existing.
follow-up: 17 comment:16 by , 5 years ago
Replying to stoecker:
- Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).
OK, good point. Should go into the "look and feel" dialog, right?
- Remember that username may be non-existing.
Does that mean that UserIdentityManager.getInstance() might return null?
follow-up: 18 comment:17 by , 5 years ago
Replying to GerdP:
Replying to stoecker:
- Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).
OK, good point. Should go into the "look and feel" dialog, right?
I'd say so.
- Remember that username may be non-existing.
Does that mean that UserIdentityManager.getInstance() might return null?
Don't know. But I do know that username is optional. You can run josm completely without OSM and I e.g. also do this.
comment:18 by , 5 years ago
Replying to stoecker:
Replying to GerdP:
Replying to stoecker:
- Add a option to disable this in the prefs. I can think of situations where this is really unwanted (e.g. when making screenshots).
OK, good point. Should go into the "look and feel" dialog, right?
I'd say so.
OK, will add a checkbox "display user name in title" with default set to off.
- Remember that username may be non-existing.
Does that mean that UserIdentityManager.getInstance() might return null?
Don't know. But I do know that username is optional. You can run josm completely without OSM and I e.g. also do this.
OK, I think the null test already handles this but I'll make sure that it works. Might take a few days as I am busy with another project.
comment:19 by , 5 years ago
v3 also implements
- add PreferenceChangedListener
- add new checkbox in "look and feel" dialog (below the "Show splash screen at startup" checkbox)
I just noticed a typo. It should be "Show user name in title", not "Show user name in tile".
The listener calls refreshTitle if either the user or the preference draw.show-user
was changed.
comment:20 by , 5 years ago
Milestone: | → 20.01 |
---|
comment:21 by , 5 years ago
Two thoughts:
1) the patch relies on the order in which listeners are notified. I am not sure if it is guaranteed that UserIdentityManager
is updated before MainFrame asked for the user name.
2) Maybe I should better evaluate the content of preference osm-server.username
instead of calling UserIdentityManager.getInstance().getUserName()
?
comment:22 by , 5 years ago
Hmm, this is quite complicated. I tested only the case that the username is changed in the preferences dialog.
If I got that right v4 will work fine in this case because UserIdentityManager
is updated by org.openstreetmap.josm.io.auth.CredentialsManager.store()
before any PreferenceChangeEvent
is created.
What confused me was that UserIdentityManager
also evaluates the PreferenceChangeEvent
.
Still, there seem to be other ways to change the field UserIdentityManager.userName
. For example, ChangesetQueryTask
may show its own dialog to set user + password and directly updates UserIdentityManager
without changing a preference.
This happens e.g. when you click "My changesets" in the "Changeset Management Dialog" in a fresh JOSM installation or if the given combination of user + password is not authenticated. Not sure why this is done.
I hope it is OK to ignore those special cases.
comment:23 by , 5 years ago
Only listening to osm-server.username
also neglects login via OAuth (which uses oauth.access-token.key
).
Maybe the cleanest solution would be to maintain a ListenerList
in UserIdentityManager
, and register the listener in MainFrame
on UserIdentityManager
. Something like this:
-
src/org/openstreetmap/josm/data/UserIdentityManager.java
a b 21 21 import org.openstreetmap.josm.spi.preferences.StringSetting; 22 22 import org.openstreetmap.josm.tools.CheckParameterUtil; 23 23 import org.openstreetmap.josm.tools.JosmRuntimeException; 24 import org.openstreetmap.josm.tools.ListenerList; 24 25 import org.openstreetmap.josm.tools.Logging; 25 26 26 27 /** … … 56 57 public final class UserIdentityManager implements PreferenceChangedListener { 57 58 58 59 private static UserIdentityManager instance; 60 private final ListenerList<UserIdentityListener> listeners = ListenerList.create(); 59 61 60 62 /** 61 63 * Replies the unique instance of the JOSM user identity manager … … private UserIdentityManager() { 96 98 public void setAnonymous() { 97 99 userName = null; 98 100 userInfo = null; 101 fireUserIdentityChanged(); 99 102 } 100 103 101 104 /** … … public void setPartiallyIdentified(String userName) { 114 117 MessageFormat.format("Expected non-empty value for parameter ''{0}'', got ''{1}''", "userName", userName)); 115 118 this.userName = trimmedUserName; 116 119 userInfo = null; 120 fireUserIdentityChanged(); 117 121 } 118 122 119 123 /** … … public void setFullyIdentified(String userName, UserInfo userInfo) { 134 138 CheckParameterUtil.ensureParameterNotNull(userInfo, "userInfo"); 135 139 this.userName = trimmedUserName; 136 140 this.userInfo = userInfo; 141 fireUserIdentityChanged(); 137 142 } 138 143 139 144 /** … … public void preferenceChanged(PreferenceChangeEvent evt) { 311 316 } 312 317 } 313 318 } 319 320 @FunctionalInterface 321 public interface UserIdentityListener { 322 void userIdentityChanged(); 323 } 324 325 public void addListener(UserIdentityListener listener) { 326 listeners.addListener(listener); 327 } 328 329 private void fireUserIdentityChanged() { 330 listeners.fireEvent(UserIdentityListener::userIdentityChanged); 331 } 314 332 } -
src/org/openstreetmap/josm/gui/MainFrame.java
a b public void initialize() { 102 104 // This listener is never removed, since the main frame exists forever. 103 105 MainApplication.getLayerManager().addActiveLayerChangeListener(e -> refreshTitle()); 104 106 MainApplication.getLayerManager().addAndFireLayerChangeListener(new ManageLayerListeners()); 107 UserIdentityManager.getInstance().addListener(this::refreshTitle); 108 Config.getPref().addKeyPreferenceChangeListener("draw.show-user", event -> this.refreshTitle()); 105 109 106 110 refreshTitle(); 107 111
comment:26 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
It is possible to change the user stored in the CredentialsManager
while uploading data.
comment:27 by , 5 years ago
Seems the approach was wrong. The user that is really in use may be stored in AbstractCredentialsAgent.memoryCredentialsCache
.
I don't yet understand if this is an error or a feature. The user changes but UserIdentityManager
is not aware of this.
comment:28 by , 5 years ago
It seems the value in AbstractCredentialsAgent.memoryCredentialsCache
is not even changed when I use the preferences dialog to set a different user. This really looks like an error.
by , 5 years ago
Attachment: | 11914.3.patch added |
---|
comment:29 by , 5 years ago
Cc: | added |
---|---|
Priority: | normal → critical |
Patch 11914.4.patch fixes the problems mentioned in comment:27 and comment:29 as long as no plugin uses
CredentialsManager.registerCredentialsAgentFactory(CredentialsAgentFactory agentFactory)
Changes:
- update
UserIdentityManager
whenCredentialsManager
is updated. - call purgeCredentialsCache() when user is changed so that we don't use previously user when the new user+password combination is bad.
@Dirk, Vincent:
I wonder why we allow a plugin to change such a basic security part. If I got that right every plugin can achive control here and send secret information to a third party? Maybe open a new ticket for this?
comment:30 by , 5 years ago
Plugins can do anything. We do not have particular security layer restricting what they can do. Remember the "no more mapping" plugin.
comment:32 by , 5 years ago
Priority: | critical → normal |
---|
comment:33 by , 5 years ago
Just for clarification: I changed the status to critical because JOSM sometimes ignored a change of the user name made in the preference dialog. I was too lazy to open a new ticket for that. Probably not a good idea...
comment:34 by , 5 years ago
Even that would not justify a critical issue to my eyes. Who really changes often its username? It's not a common scenario.
comment:35 by , 5 years ago
OK, it's unliky to happen but when it happens its effect may be that you upload a changeset with the wrong user. I consider this critical, esp. when JOSM shows that you work with user abc and in fact you upload as user xyz.
comment:36 by , 5 years ago
Well for JOSM the states are:
- normal - all
- major - more important issue affecting a group of users
- critical - a large number of users is affected (we're getting a lot of duplicate reports), usually results in a hotfix and manual latest compilation
- blocker - JOSM doesn't work at all for a majority of users
Example titlebar OSM username