Local admin accounts #332
Merged
Local admin accounts #332
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Still need to implement tests and cleanup various things, but the basic pieces are integrated.
Other fixes for build process to ensure assets always get precompiled during build and in production mode.
- Use devise-i18n to provide translations for the various default Devise views. - Update the Accept-Language header detection to take into account region (so "es-419" is properly handled now that devise-i18n also has "es" data). - Update the existing i18n tests to account for the new server-side content on the default login page. We'll no longer try to read the i18n files directly, and instead put the expected i18n values directly in the test script (since otherwise, it's tricky to try and fetch the devise-i18n default values from within the test suite).
Since adding the new login process with local accounts, we had flipped the ApplicationController into "exception" mode for CSRF protection. However, this broke our POST/PUT API endpoints, since those requests won't have the CSRF token (we're relying on API keys and admin tokens instead). So this restores the previous behavior of just nullifying the session for API-specific endpoints instead of throwing exceptions when the CSRF token is missing.
This got changed around during the Rails 4.2 upgrade. Also remove no longer used DEVISE_SECRET_KEY environment variable reference.
- Consolidate where "rails_secret_token" is set for the test environment, so it behaves more like production and can be read more easily for tests. - Fix validation error message display to restore more consistent behavior between client side and server side, since we switched server side to use i18n full_messages. - Fix client-side validation errors not displaying inline on the form after hitting submit. - Get rid of unused "devise_secret_key" setting after Rails 4 upgrade. - Upgrade to released version of mongoid-embedded-errors gem that has Mongoid 4+ compatibility. - In the admin auth endpoint, just return the needed metadata about the current admin user. This prevents us from returning a bunch of other things, like the encrypted_password attribute to the client. - Fix server side validation response using full_messages leading to an error because mongoid-embedded-errors and full_messages was trying to set new keys in the errors hash while we looped over it. - Add more tests for the validation display handling. - Fix various tests due to the local admin changes.
This isn't quite the same as "npm prune" that we were replacing. "clean" will remove files from inside the npm directories that it thinks are inessential. But this doesn't always work.
- More thoroughly implement the concept of the "username_is_email" option that hides the separate email input when true. - As a result, change a few things about how we share i18n data being server-side and client-side so we can dynamically override the i18n "username" text with the "email" text if this setting is true. - Various fixes for when local authentication is disabled, like disabling the login HTTP POST endpoint. - Bump default minimum password length to 14 chars.
- Allow email invitations to be sent for new admin accounts created. - The email invite process allows new admins to set their own password when using the local authentication mechanism. - Remove devise_invitable, since our concept of invitations isn't quite the same as that gem's (I was thinking the gem could be used for this when I added it, but our needs are actually simpler). - Cleanup some things around Devise's "trackable" plugin for admins. We were previously referring to the "last_*" attributes (eg, last_sign_in_at), but it turns out this isn't really correct for what we want. The last known login session is actually under the "current_*" attributes instead (last_* is for the previous). So shift most of our displays for "last login" to use the "current_*" attributes. - Better align our last provider tracking with these updates. We weren't previously keeping track of both the current and last, so now we keep track of both. Also implement it in such a way that we can track the provider for the "local" admin accounts.
- Use a custom LDAP login form so CSRF token is passed along. - If LDAP is the only login option, put the login on the initial page. - Use the LDAP "title" on the various pages and forms, so things can read "Sign in with Company Name" (instead of the generic "LDAP"). - Make email optional so when the username is not the email address, we can still login and create accounts from LDAP when all we know is the username (since LDAP may not provide email). - Add integration testing for LDAP authentication with simple openldap server.
This was left over from when we thought we might be using devise-invitable, but we've since removed that.
- Ensure we don't delete all the admin accounts for tests that run in parallel, since that may impact other tests. - Ensure at least 1 admin account exists prior to some admin routing tests, so the admin doesn't redirect to the screen allowing for admin creation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #314. Essentially, with the demise of Persona, having an easier to setup login option for the admin seems more important (this request had also come up previously for other reasons).
This implements a local admin account option using Devise. However, third-party logins (using Google, GitHub, etc) is still available. From an upgrade perspective, the main thing to be aware of is that the
web.admin.auth_strategies.enabledsetting now defaults to just use the newlocaloption. If you'd like to continue using third-party login options and you hadn't already overridden this setting, you'll need to explicitly define it in your/etc/api-umbrella/api-umbrella.ymlconfig file to enable your desired login options:Other notes:
web.admin.initial_superusers(although that option is still supported and might be use for production deploys where you want to ensure specific accounts exist, rather than relying on this first-time setup flow in the interface).web.admin.username_is_emailoption which we default totrue. When true, this hides this internal distinction between usernames and e-mail addresses in the interface and keeps the fields in sync. If set tofalse(for providers like LDAP or CAS where there might be an explicit username), then the e-mail and username fields will both show up separately.