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

Skip selecting session #57

Open
pfsalminen opened this issue Jan 31, 2024 · 21 comments · May be fixed by #81
Open

Skip selecting session #57

pfsalminen opened this issue Jan 31, 2024 · 21 comments · May be fixed by #81
Labels
enhancement New feature or request

Comments

@pfsalminen
Copy link

I would like the option to bypass the first User/Session selection window by default via a config settings.

Desired behaviour:

  • On startup, the password screen would be shown first
  • Session and User would be set to the last used or default settings
  • Hitting "Cancel" brings back the User/Session selection window
@rharish101 rharish101 added the enhancement New feature or request label Mar 12, 2024
@max-ishere
Copy link
Contributor

As a Nix user I would appreciate a config option and a separate cache file. The config setting should disable cache.

@dante-e-v
Copy link

I would amend it and say it should default to remembering the last username and session and go right to the password input.

This would be similar to using tuigreet with the flags --remember --remember-user-session.

This is the default behavior of most greeters and it's understandable since it lets the user start the session as quickly as possible without having to select anything.

If someone wants to configure regreet to use the current behavior they should be able to in the regreet.toml file.

P.S. If OP is saying this I'm just trying to clarify as from what was said it sounds like they want the option for remembering user sessions that can be configured in regreet.toml rather than that being the default behavior. I've already explained my reasoning on why.

@pfsalminen
Copy link
Author

@dante-e-v I suggested making it an opt-in setting to not change expected behaviour by default, but I do like the idea of making it the default as you suggest.

@rharish101 rharish101 linked a pull request Aug 4, 2024 that will close this issue
@rharish101
Copy link
Owner

Hello everyone, could you test #81 and check if it works for you? I haven't made it the default, but made this opt-in in the config.

@max-ishere
Copy link
Contributor

Im gonna try it on nixos today, hope it compiles.

@max-ishere
Copy link
Contributor

Uhhh, not gonna spend any more time on this. First off, it crashes on start because you can't have short='l' twice: https://github.com/rharish101/ReGreet/blob/c55bfbc8c08b730d8e556d100c454dcb97902c20/src/main.rs#L44C4-L50C25

and also I cant see this behavior in demo mode easily so...

What I need:

  • Fix clap (I have done it manually, but still)
  • Explanations on how to get this working in demo mode. Like what args/configs I need to create to see the new behavior.
  • (flake.nix, but I have a feeling I might have to pr this one myself/someone else)

@max-ishere
Copy link
Contributor

Ok so, after digging through literal spaghetti code (I am sorry, there's too many side effects, its the honest truth) I have managed to make the greeter open the password screen, however, I have no idea if this is even gonna work outside the demo mode (which is pita to test).

@max-ishere
Copy link
Contributor

I have one concern: what if there's no password required for a cached user and the greeter just logs in to the session, effectively locking the system to a single user.

I will have to spend probably about another hour digging through code because there's too many places where some variable is set and then some completely other place reads it and then, and then, and then...

@rharish101
Copy link
Owner

I have one concern: what if there's no password required for a cached user and the greeter just logs in to the session, effectively locking the system to a single user.

That's a good point. When the greeter starts, it doesn't know if the user needs a password or not before it asks greetd to log in with that user. At this point, for a passwordless user, it would auto-login.

I guess I could check /etc/passwd to see if the user has a password set, but maybe it wouldn't work if a user has nothing other than a fingerprint reader or some other auth method setup.

@max-ishere
Copy link
Contributor

Uhhh, that doesnt sound like a good answer if you're using Relm. Instead how about the UI is refactored to be actual Elm architecture and then you replace this one line:

-self.login_button.emit_clicked();
+self.app_model = AppModel::PasswordScreen{ ... };

@max-ishere
Copy link
Contributor

I've created #85 to discuss UI refactors there and not clutter this issue with unrelated talk

@max-ishere
Copy link
Contributor

I looked at the docs for greetd and it looks like you can just not start a session after you create it. So basically if cached -> create_session() if success then wait for login button to be pressed and then start_session()

@max-ishere
Copy link
Contributor

max-ishere commented Aug 27, 2024

What I need:

  • Fix clap (I have done it manually, but still)

#87

@max-ishere
Copy link
Contributor

What if we join the user/session selection screens into one? Then there would be no need to skip anything. Here's how it will work:

If config.start_session_on_startup then go to screen 2. And if session can be started then show the ui to autologin. Else go to screen 1.

Screen 1

   User: [____________]
Session: [____________]
                (Login)

Screen 2

   User: [____________]
Session: [____________]
***********************
******  AUTH UI   *****
***********************
       (Cancel) (Login)

Auth UI

Will contain eiher a large bold label for Info, red-border-label for Error, Entry for Visible and PasswordEntry for Secret. Question class auth ui will also have a label above them for the greetd message.

Here's a long OTP Example:

   User: [____________]
Session: [____________]

Very long OTP query
mentioned in some other
PR would look something
like this
[_____________________]
       (Cancel) (Login)

In case session can be started at startup example:

   User: [____________]
Session: [____________]

      Autologin
       (Cancel) (Login)

@max-ishere
Copy link
Contributor

Also if on screen 2 the session is changed, that change will be used when starting the session. And when the user is changed, then the old session will be canceled and a new with that user will be started.

@max-ishere
Copy link
Contributor

I am currently in the process of making this kind of UI, but given my current schedule there's no ready date.

@max-ishere
Copy link
Contributor

Screenshots of the UI I proposed in text blocks. I use the smallest width because it is the easiest to get a consistent width on.

Autostart prompt (for addressing the autologin lockout, not shown under any other conditions)
image

Authentication not started yet
image

Password prompt
image

Visible prompt
image
this one affects the width, I dont think it will be a hard fix, just not rn.

Informative prompt
image

Error prompt (not IPC IO errors, specifically for showing AuthMessageType.Error
image

@max-ishere
Copy link
Contributor

max-ishere commented Aug 30, 2024

And I know what some that know how the UI used to work think, if you change the user, then what? Then, the UI goes to the "Authentication not started yet" state.

Edit: Or it could cancel the prev. session and start a new one automatically.

@max-ishere
Copy link
Contributor

And if the session changes, then nothing in particular happens. it just saves the selection to be used when the session is started.

@max-ishere
Copy link
Contributor

max-ishere commented Aug 30, 2024

If anyone wants to test it out: https://github.com/max-ishere/ReGreet/tree/703962cb5e644b14b8d7dcb66d2844a91bcf9824

Edit: Oops, forgot to git add all the changes...

@max-ishere
Copy link
Contributor

⚠️

Just realized is should have mentioned that the link above is just the UI, it doesn't actually interact with greetd IPC. So launch it in demo mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants