[WIP] feat: add authentication

[JulienAmoros]

🚧 WIP - Add authentication

Description

This PR purpose is to add devise in order to link new passwords with connected user in order to be able to retreive a password link, expire or delete password.

Changelog

• ➕Install Devise
• ✨Add User controller with passwords action to see your passwords (when being connected)
• ✨Added buttons to ease navigation
• 🎨UI Polishing for Passwords views and Views views
• 🔒Views and password controller as been reworked such as a user can only get informations regarding his.her passwords.

User stories:

AS a user, I WANT to be able to see previously created passwords SO THAT I can retreive a lost link.
AS a user, I WANT to be able to see previously created passwords SO THAT I can expire some.
AS a user, I WANT to be able to see what IPs/UAs viewed a particular password that I pushed.

Migration

YES

Looks like there where already Devise installed long time ago, I decided to drop corresponding table to regenerate Devise model.

Notes

• To sign up, go to /users/sign_up
• To sign in, go to /users/sign_in
• Please check config/initializers/devise.rb to make proper setup
• I don't know how you manage secret key base, so I added config/secrets.yml, you will have to add this env var on production: SECRET_KEY_BASE

TODO

• Button to expire a password
• Navbar to connect
  • Add buttons/links to sign_up / sign_in
• UI polishing
• Make a proper Devise configuration
• Check that migration are not making any trouble
• Check there are no regression

Questions

@pglombardo
Can you give me your opinion about:

• this functionnality by itself
• the code I made
• UI/UX you would like to seeon this

[pglombardo]

This is great. I've had this planned for quite some time but never got to it because of the scope.

Two items to note:

1. This will also have to be supported in the API somehow - potentially a feature request/TODO for the time being
2. When I originally planned this feature, I added a views table that tracks IP and browser user-agent for each password view. This way password publishers can view what IPs/UAs viewed each password. (e.g. then having the option to expire the password) Showing views for a particular password in the dashboard would be very helpful.

As for UI, I have no objections as long as it maintains simplicity and is consistent with the theme.

AS a user, I WANT to be able to see previously created passwords SO THAT I can expire some.
AS a user, I WANT to be able to see previously created passwords SO THAT I can delete some.

We support expire (deletion of the password) but not deletion of the entire record so that secret URLs can still show expired state.

Then to add:

As a user, I WANT to be able to see what IPs/UAs viewed a particular password that I pushed.

So far it looks great. I'll take another look at the code again as this progresses.

[pglombardo]

We will also need a configuration option to make authentication required or optional per #84 . I can add this also after the fact if needed.

[JulienAmoros]

@pglombardo Ok, there you have a little update:

• ✨Added buttons to ease navigation
• 🎨UI Polishing
• 🔒Views and password controller as been reworked such as a user can only get informations regarding his passwords.

Here you have the password list for a user /users/pwds:

And there you have the list of views for a particular password /users/views/:id:

NB: The last screenshot is exactly the same for /users/views which will list all views of all passwords belonging to the logged in user.

Functionality Scope

I think making this work with the API is out of the scope of this PR, this functionnality is already big enough in my opinion.
For the configuration part regarding #84, I think we can add environment variable enabling or disabling before_action :authenticate_user!, with condition on a lambda, on Passwords controller, something like this maybe:
before_action :authenticate_user!, only: [:new], if: -> { ENV['ONLY_USER_CAN_CREATE_PASSWORD']=='true' }
I'll let you do this one, as long as you are managing production env 😉

Next Step

Now the question is, what do you think of this?
Where do you want to put buttons / links to log in, log out and edit account?
I think the main page is already filled with a lot of informations, and that some are more belonging to an "About" or "FAQ" pages than on main page.

[pglombardo]

This is great progress @JulienAmoros. Give me a bit to free up some time and catch-up.

[pglombardo]

A few changes/fixes requested.

[pglombardo]

I think making this work with the API is out of the scope of this PR, this functionnality is already big enough in my opinion.

Agree

For the configuration part regarding #84, I think we can add environment variable enabling or disabling before_action :authenticate_user!, with condition on a lambda, on Passwords controller, something like this maybe:
before_action :authenticate_user!, only: [:new], if: -> { ENV['ONLY_USER_CAN_CREATE_PASSWORD']=='true' }
I'll let you do this one, as long as you are managing production env 😉

Cool - we can address this afterwards.

Now the question is, what do you think of this?

♪┏(°.°)┛┗(°.°)┓┗(°.°)┛┏(°.°)┓ ♪

I think the approach is perfect - minimalist and functional. We can touch up as things progress.

Where do you want to put buttons / links to log in, log out and edit account?
I think the main page is already filled with a lot of informations, and that some are more belonging to an "About" or "FAQ" pages than on main page.

Agree - post merge I'll likely do some touch ups and move some of that stuff off of the main page.

[JulienAmoros]

@pglombardo Should I leave this trackable part or not if we actually want to know the fewest about users?

[JulienAmoros]

Hey @pglombardo !
I worked a bit on this, here are small (not exhaustive) changelog:

• 🔧Deleting ability for a user to be Recoverable and Confirmable
• ⚙️User's email attribute as been changed to username
• ⚙️Devise view have been generated and slightly modified in order to change email to username

And here are new questions:

• Should I erase "Trackable" part on user migration as we want to know less about users?
• Can you try migrations on a duplicate of your database in order to ensure they don't break anything? (I raise your attention on drop_table :users action on my migration in order to get rid of previous useless User table)
• When a password expire, should we be still able to see it in Passwords list (but with an information that says it's expired or smth)?
• When a password expire, should we be still able to see Views that occured on it?
• Once finished, should I clean auto-generated comments in initializers, models, migrations... ?
• Once finished, should I delete empty files that has been auto-generated? (empty helper, empty coffee scripts...)
• Once finished, should I delete useless files that has been auto-generated? (like unused devise views for example)

Also, I rebased on you master branch.

I look forward for your answer 😄

[pglombardo]

Hi @JulienAmoros - So sorry that I've been late in replying to this... #startuplife. I'm going to run through some testing locally on this and will post back here this week.

[pglombardo]

Took a look at this last night. There might be a problem with the migrations as it says no username field but let me take a closer look over the weekend.

• Should I erase "Trackable" part on user migration as we want to know less about users?

Trackable should go away if possible or later on if it's easier.

• Can you try migrations on a duplicate of your database in order to ensure they don't break anything? (I raise your attention on drop_table :users action on my migration in order to get rid of previous useless User table)

It's fine to drop/recreate the users table since we weren't using it anyways. I'll retry this weekend and before this upgrade, we'll test it on more dbs, have backups just in case. Most important is that pre-existing URLs still work. If just users table, the risk is low to none.

• When a password expire, should we be still able to see it in Passwords list (but with an information that says it's expired or smth)?

I would say yes with a note saying essentially the password is expired and no longer there - so we can still see view logs and have historical record.

• When a password expire, should we be still able to see Views that occured on it?

Yes - same as above.

• Once finished, should I clean auto-generated comments in initializers, models, migrations... ?

Not sure - I'll take a look but might not be necessary unless the info is incorrect/misleading.

• Once finished, should I delete empty files that has been auto-generated? (empty helper, empty coffee scripts...)

If they have no future, I would delete. If future features may use them, then it doesn't hurt to leave them in. Aiming also for simplicity for other potential contributors - so judgement call.

• Once finished, should I delete useless files that has been auto-generated? (like unused devise views for example)

I would say yes to this. Also as long as we don't break any users upgrading from very old versions.

[cb3inco]

What happened with this?

[JulienAmoros]

@cb3inco Hi, I never took the time to finish this. But if you want to take over, don't hesitate 😃

[cb3inco]

Oh no worries. I'm by no means a developer, just an app user. Unfortunately I won't be of much help. 😢

[pglombardo]

After > 3 years this is now done/implemented and live on pwpush.com. Thanks @JulienAmoros for taking a shot at this!