NAS-127183 / 24.10 / Customer designated login banner #13892
Conversation
bugclerk
changed the title
|
Is there any reason to not just always display motd before login? |
On the WebUI it could get annoying to have a popup modal every single time you log in with the motd. For the SSH part, we could do that but might as well keep parity with the WebUI for that logic. |
|
Instead of parsing the SSH session, would verifying that the SSHd config contains the banner line be sufficient? As well as looking to see that the contents of /etc/motd is also what we just set. |
|
Have a passing test! |
|
Doesn't look like any alembic changes were made actually, I was under the impression that this PR had DB changes. Should be clear to just merge as is. |
There was a problem hiding this comment.
Hmm, I don't think adding a new db column is necessary. I'd rather keep this as simple as possible. We should just generate SSH to show the motd if one is set in the database. I don't want to have another boolean option that enables this functionality.
To summarize:
- if
adv_motdis set, then generate ssh accordingly. - let's not call it
get_motd. This is actually being used as a login banner. So we can name itlogin_banner. return datastore.query, system.advanced, [], {'get': True})['adv_motd']
|
I added the checkbox as it was suggested in the ticket! The reasoning (from what I assume) is to prevent having a popup every single time you log in (although we could just have it show up on the login page in general, not as a modal, but then there's UI flow issues with large banners). |
|
@anodos325 how would we go about deleting the old /etc/login_banner if it's in its own mako? If exiting early stops the file from being written, then I could probably do a delete then exit early. |
raise FileShouldNotExist within mako file (you will need to import from etc plugin) |
|
@anodos325 I've implemented those changes! Is there any way to specify that when we're reloading the ssh service that we should etc generate our login_banner as well? Would save us a line in the |
I'm looking into this right now |
If you put the login banner in the etc group for |
|
The no_authz_required issue was me putting the accepts decorator above the no_authz_required, moving it below fixes it. Should be good for final review now! |
src/middlewared/middlewared/etc_files/local/ssh/sshd_config.mako
Outdated
Show resolved
Hide resolved
| @@ -297,6 +297,7 @@ class EtcService(Service): | |||
| {'type': 'mako', 'path': 'pam.d/sshd', 'local_path': 'pam.d/sshd_linux'}, | |||
| {'type': 'mako', 'path': 'local/users.oath', 'mode': 0o0600, 'checkpoint': 'pool_import'}, | |||
| {'type': 'py', 'path': 'local/ssh/config'}, | |||
| {'type': 'mako', 'path': 'login_banner', 'mode': 0o600}, | |||
There was a problem hiding this comment.
Also need to add system.advanced.login_banner to ctx above so that we only query once and provide to all scripts in this group.
Still need to make sure the tests run fine though
| import os | ||
| import pytest | ||
| import sys | ||
| apifolder = os.getcwd() |
There was a problem hiding this comment.
You don't need to do this. Just need to import the relevant modules.
|
This PR has been merged and conversations have been locked. |
Government systems require a login banner prior to being allowed to log in. In the ticket it was determined to reuse the motd as our banner.
This PR has the middleware portion of this ticket. On top of giving a public endpoint for motd, we also have our motd_before_login option, which updates our sshd configuration to show our MOTD prior to login. Our database string length has also been upped from 1024 to 4096 (DoD requires 1300 minimum to fit the bare minimum message).