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

Default to using InvitationKeys model for locking down a server #1452

Open
swombat opened this issue May 10, 2024 · 7 comments
Open

Default to using InvitationKeys model for locking down a server #1452

swombat opened this issue May 10, 2024 · 7 comments

Comments

@swombat
Copy link

swombat commented May 10, 2024

Having to restart the server each time you want to change the invitation keys is a bit clumsy.

A simple solution I've implemented is to define a very simple InvitationKeys model.

rails generate super_scaffold InvitationKey Team key:text_field

Then, redefine the invitation_only? method in bullet_train.rb:

# Override to use keys instead of ENV
def invitation_only?
  InvitationKey.all.any?
end

And then eject the invite_only_support.rb concern to replace the invitation method with:

  def invitation
    return not_found unless invitation_only?
    return not_found unless params[:key].present?
    return not_found unless InvitationKey.all.collect(&:key).include?(params[:key].strip)
    session[:invitation_key] = params[:key]
    if user_signed_in?
      redirect_to new_account_team_path
    else
      redirect_to new_user_registration_path
    end
  end

This way, the first user to sign up can add an invitation key via their dashboard, and the moment they do, the app is locked down by invitation code. If they remove all invitation codes, the app is open to free signups again.

If the dev wants to make this invitation_keys table visible only to the "site admin" they can code that themselves.

I'm not sure:

  • Whether this is a desirable core feature
  • Whether there is a better way to implement it (maybe a rake task that you can execute to enable invitations?)
  • If it's a desirable core feature, how to implement it in core

Open to discussion.

@jagthedrummer
Copy link
Contributor

@swombat can you talk a little more about your use case here? Generally invitation keys (as they're implemented today) are designed to be something that the developer sets up, they're not intended to be created by end users. I'd be interested in hearing about whatever you're doing that has you changing them frequently enough for this to be a problem you'd like to solve.

@swombat
Copy link
Author

swombat commented May 18, 2024

Well, Invitation Keys are an early concern for an app, so that would argue for them not having a long-term model... but (at least for me) the natural use is you set up a key for a given user and you then send them the link. They use it, and then possibly you remove the key to prevent it being shared around.

For example, I set up specific invite keys with someone's name in them. And someone just invited me to a BT app and the invite key was a long hexadecimal string.

The consequence of this is that invite keys change. With the default setup, each time they change, the server has to be restarted. That's not great. They also can't be created or removed easily - you have to mess with the server's environment variables.

To me the obvious solution to this was to have a convention (likely true) that whoever signs up first can lock down the signup process by adding at least one invitation key, and can edit those in the app. In my apps I end up making this model only visible to the site admin team id, but that would require shoehorning another concept (site admin) into BT so maybe not viable (though my elegant hack for this is it's specified by an env variable - or the very first team to sign up if the env variable is empty).

This avoids the issue of restarting the server whenever invite keys are issued.

@jagthedrummer
Copy link
Contributor

Thanks for the deets, @swombat. I don't think the current implementation is intended for the use-case that you're describing. I think it's really intended more for a relatively short beta period where you'd be giving a single shared invite code directly to somewhat trusted beta testers.

I wouldn't be opposed to exploring if we can offer something closer to what you're describing, but my initial gut reaction is that this kind of thing is going to vary quite a bit from app to app, and might be more of an "app concern" than a "framework concern".

You already touched on the idea of whether "site admins" are the only ones who could make invitation codes, and I think there are lots of related questions that would be hard to answer in a general "this should work for everyone" kind of way. Stuff like:

  • Can people who have already gotten in generate codes for their friends?
  • If so how many can they generate?
  • Do they get them all at once, or on some sort of schedule?
  • If current users don't generate whatever codes they're allowed to generate, does the opportunity to generate them expire?
  • Are all codes one-time use?
  • Do codes expire if they're not used within a certain time period?
  • Etc...

@gazayas @kaspth @pascallaliberte @andrewculver, do y'all have any thoughts on this?

@kaspth
Copy link
Contributor

kaspth commented May 20, 2024

@swombat interesting! I don't know if I'm understanding the use-case correctly, because I wonder if this could be accomplished with a feature flag via something like Flipper/Flipper Cloud?

Then you could grant the :sign_up feature to a particular User or Invitation or Membership (not sure which of the 3 it ought to be), or something like that?

Once the private :sign_up phase is settled, you can enable the feature flag for everybody, so everybody can sign up and then you don't need any homegrown models to manage/implement/migrate/drop.

@gazayas
Copy link
Contributor

gazayas commented May 21, 2024

@jagthedrummer @swombat

but my initial gut reaction is that this kind of thing is going to vary quite a bit from app to app, and might be more of an "app concern" than a "framework concern".

I definitely agree that this is more of a framework concern than an app concern, and this may not be a fit for BT ideology especially since the uuid keys that we issue out when sending an invite to a unregistered user (in a standard BT app without invitation_only? enabled) already serves as a way of generating a membership exclusively for the user who has the uuid key. As @jagthedrummer stated, and at least the impression I'm under is that Invitation Only mode is more for a beta period of the app itself as opposed to a configurable feature available to users.

HOWEVER, I don't think that @swombat's idea is completely off/wrong, and I just think that a big part of why it's a framework concern for us is because of the Team setup that comes stock in BT. When a new User is registered, they're required to belong to a Team, and if they haven't been given an invite, they get a default Team with admin abilities. If the app is eventually freed up to everyone they'll still be able to create their own Team with admin abilities (as opposed to only being a User that doesn't have a Membership or Team), so the question is whether that's desirable or not. Maybe I'm not understanding the use-case right now, but if it were to be implemented, that's the first hurdle that comes to mind.

We already have a exclusive member-identifying key setup with invitations for the main functionality of BT to protect the Teams themselves, so maybe I just need to see a more specific example to understand the use-case.

With that being said, I don't think we have to throw away @swombat's idea completely, but we could potentially put a compatibility layer on top of what we have so to speak as opposed to reworking the User registration (or Invitation Only mode) logic.

Like @kaspth said,

Once the private :sign_up phase is settled, you can enable the feature flag for everybody, so everybody can sign up and then you don't need any homegrown models to manage/implement/migrate/drop.

That might be the first direction we want to go in to make things configurable for Users.

@swombat
Copy link
Author

swombat commented May 21, 2024

I mean, an invitation system can be as complicated as you can dream it up... 😅

It feels to me that since BT has any invitation system it already has accepted the idea that this is a SaaS framework concern.

That said I don't think that means that BT needs to handle all possible edge cases (expiration keys, enabling invitees to invite more people, etc)... but imho it does need to be a little bit more functional than it is now.

I wanted to give each person an individualised invitation. And no, I don't want to invite them in my team, I want them to be able to sign up to their own team. It was quite frustrating with the default BT system to have to restart the server each time I changed the invitation codes.

FYI I have made a minimal invitation system as a generator here, since I was about to code up this exact same thing on the third site in a row now:

https://github.com/swombat/swombat_tools/blob/main/lib/generators/swombat/invitation_keys/invitation_keys_generator.rb

It doesn't even need the site_admin? concept. This just creates an InvitationKeys model that every team has access to, and if the specific app wants to lock that down it's easily hidden with a site_admin concept (which imho should be a framework concern, but that's another thread).

@swombat
Copy link
Author

swombat commented May 21, 2024

Oh and the new method didn't even need touching the concern. Weepee.

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

No branches or pull requests

4 participants