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

Add authentication via devise #625

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

murny
Copy link
Collaborator

@murny murny commented Aug 15, 2024

This is the first of a few PRs to help replace the basic authentication we have for CMS Admin (more info here #551)

In this PR we added specifically:

  • Devise which is a popular ruby gem for managing authentication for users stored in the database.
  • Admin table which will hold the users data
  • Configuration and setup of Devise
    • we removed "Registerable" concern as we don't want users to "Sign up" new accounts
    • we added the "Trackable" concern as we want to have some extra metadata on user sign ins
    • Everything else is pretty much the default configuration of Devise

Here is a quick overview of what this gave us:

When you first go to http://localhost:3000/admin to view the Admin area of the CMS you will get booted to the login screen:

image

You can login via the user we created in the seeds, on this form use the email: "[email protected]" and password: "password" and hit the login button.

You will now be redirected to the Admin CMS dashboard:
image

That's it. This replaces the Basic auth flow, and we now have personalized Admin accounts in the database which people can log into the Admin CMS.

Some improvements could be:

  • Fix broken tests, switch from my basic auth helper to devise test helper and update tests
  • Add more tests for admin model and critical sign in flow (if needed)
  • Style the login page better (might need some margin/padding on the top of the form, etc)
  • Style the flash messages for successfully signing in
  • Handle error state in the sign in form (I assume we not rendering flash messages here in our application layout)
  • add ability to logout (I have added this in Add admin crud UI #626 )
  • Devise includes a "forget password?/reset passwords" workflow. This does work. However, will probably need some configuration in Production as we need to be able to send emails. Also the views for these forms will need some love just like the sign-in form (padding/margin, styling of error messages/flashes etc).

Future work:

.bundle/config Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started to encounter some weird issues with installing mysql gem in this repo. Not sure if this is required anymore?

# Configure the e-mail address which will be shown in Devise::Mailer,
# note that it will be overwritten if you use your own mailer class
# with default "from" parameter.
config.mailer_sender = "[email protected]"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be updated to something real (I added a note in the PR description for needing to setup emails in production)

@@ -1,6 +1,9 @@
# frozen_string_literal: true

Rails.application.routes.draw do
root controller: "comfy/cms/content", cms_path: "", action: "show"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for making root_url and helpers work in comfy mexican sofa. Devise will often use root_url for various things.


require "test_helper"

class AdminTest < ActiveSupport::TestCase
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea to add some tests here and maybe some higher level tests to test the Sign up flows/etc.

We will have to address the failing tests as well in this PR as I have switch away from Basic Auth to Devise and will need to update the failing tests to use the devise sign in test helpers.

@murny murny mentioned this pull request Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant