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

App-Pins menu vanished partially from "personal settings" menu #186

Closed
ho1ger opened this issue Aug 10, 2017 · 14 comments
Closed

App-Pins menu vanished partially from "personal settings" menu #186

ho1ger opened this issue Aug 10, 2017 · 14 comments

Comments

@ho1ger
Copy link

ho1ger commented Aug 10, 2017

Since the update to NC 12.0.1, the App Pin menu in Personal Settings -> Security -> App Pins vanished partially. I still see the textbox + "Create New Password" button that you use for creating a new App Pin but the list that should show already existing App Pins and would allow their revocation vanished. Besides this UI issue, old and newly created App Pins work fine.

bildschirmfoto 2017-08-10 um 21 25 12

TOTP version is 1.3.0.

I verified this issue on my second nextcloud instance.

@ChristophWurst
Copy link
Member

Hi,

looks like this is a duplicate of nextcloud/server#4535 (app passwords are a feature of the Nextcloud server and not this app).

Which browser did you use?

@ho1ger
Copy link
Author

ho1ger commented Aug 11, 2017

Oh sorry, I did not expect that.

I verified this in Safari, Chrome, and Firefox. And I only have locally defined users, so no LDAP backend used.

@ChristophWurst
Copy link
Member

Interesting. Could you please check your browser's developer console, especially in the network tab? You should see a request for the tokens. I'm wondering whether that returns the correct data. Knowing that would help us narrow down the bug. At the moment I'm not sure whether it's just an issue with the front-end or something on the server-side.

(I'm having issues with my dev env at the moment nextcloud/server#6078 (comment) - will try to investigate/debug this soon).

@ChristophWurst
Copy link
Member

bildschirmfoto von 2017-08-11 09-49-45

(Request goes to https://localhost/settings/personal/authtokens and returns HTTP 200 and some JSON data here)

@ho1ger
Copy link
Author

ho1ger commented Aug 14, 2017

Hi,

sorry for the delay, got sick.

I tested in Chrome; it looks like that the call returns various authtokens but they are not rendered by the UI.

bildschirmfoto 2017-08-14 um 10 30 04

@ChristophWurst
Copy link
Member

I tested in Chrome; it looks like that the call returns various authtokens but they are not rendered by the UI.

Perfect, thanks. This tells me that all seems to be fine on the server-side an it's indeed just the UI part that behaves unexpectedly.
Could you please check your browser's developer console for js errors or warnings?

@ho1ger
Copy link
Author

ho1ger commented Aug 14, 2017

Hmmmm, ... hope this is what you need. I tested both instances and interestingly they behave slightly different:

bildschirmfoto 2017-08-14 um 12 22 09

bildschirmfoto 2017-08-14 um 12 22 17

What I do not understand is which script the debugger is referring to. Furthermore I get the same error when I open any other page, e.g., the file browser or contact app.

@ChristophWurst
Copy link
Member

Indeed interesting that two instances behave differently. Which language are you using? Maybe this bug is related to the l10n system. The warning shown on the screenshot seems to originate from here https://github.com/nextcloud/server/blob/95b7fa306ef786b43286abdcab26916c503e4ec6/core/js/l10n.js#L99.
cc @nickvergessen

@ho1ger
Copy link
Author

ho1ger commented Aug 14, 2017

German. I set both instances to English but the missing auth tokens do not show up. However, the error about the missing plural form disappeared. hmmmmm...

@ho1ger
Copy link
Author

ho1ger commented Aug 15, 2017

Updated to NC 12.0.2 + TOTP 1.3.1: problem still exists.

@ghost
Copy link

ghost commented Aug 31, 2017

Same here on two different installations. Do you require additional information to tackle this issue?

@ChristophWurst
Copy link
Member

@ho1ger apparently the combination of active sessions and app passwords since nc12 caused some confusion for users as commented at nextcloud/server#6075 (comment). Are your app passwords shown in the list of devices/sessions?

@ho1ger
Copy link
Author

ho1ger commented Sep 5, 2017

@ChristophWurst Orrrrr... ;-) Now I understand the problem. I did not know/notice that the both views got combined. Now when I hover over the sessions list I get my three ... that I can use to invoke the context menu that allows me to delete an App Pin. To answer your question: no the passwords are not shown. But I guess this is intended as this functionality didn't exist before the combination of both views.

Let me explain why I think this combination is not a good and intuitive idea. 1) Imho a session and an App Pin are entirely different concepts. For me an App Pin is something quite persistent that typically lives as long as you want to use a device. A session, however, is a communication relationship between a server and a device that is created and destroyed many many times. 2) Furthermore, not all sessions can be associated to an App Pin; only those sessions started by an app that doesn't support TOTP.

Hence, I never got the idea to search App Pins in the sessions menu.

I suggest to mitigate the issue by permanently showing something in the sessions list that indicates that the list has more capabilities than being a list of sessions. Still I think the combination is not really logical and it might confuse people.

@ChristophWurst
Copy link
Member

Great that we could resolve this mystery :)

To answer your question: no the passwords are not shown. But I guess this is intended as this functionality didn't exist before the combination of both views.

That's what I meant. Yes, of course the passwords are not shown, but you should see an entry for each password :)

Let me explain why I think this combination is not a good and intuitive idea. 1) Imho a session and an App Pin are entirely different concepts. For me an App Pin is something quite persistent that typically lives as long as you want to use a device. A session, however, is a communication relationship between a server and a device that is created and destroyed many many times. 2) Furthermore, not all sessions can be associated to an App Pin; only those sessions started by an app that doesn't support TOTP.

Hence, I never got the idea to search App Pins in the sessions menu.

I suggest to mitigate the issue by permanently showing something in the sessions list that indicates that the list has more capabilities than being a list of sessions. Still I think the combination is not really logical and it might confuse people.

Totally valid points. Thanks a lot for your feedback. We can definitely improve this. Could you please file a ticket for this at the server repo? I'm sure @jancborchardt also has some ideas on how to improve this. Maybe all it needs is a better explanation text on those settings page and different layouts/icons to differentiate between sessions and app passwords.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants