Skip to content

Disable user registration by default#5328

Closed
cpoetter wants to merge 1 commit intogetsentry:masterfrom
cpoetter:master
Closed

Disable user registration by default#5328
cpoetter wants to merge 1 commit intogetsentry:masterfrom
cpoetter:master

Conversation

@cpoetter
Copy link
Copy Markdown

@cpoetter cpoetter commented May 2, 2017

As the following pull requests and issues show, disabling the user registration seems to be one of the most wanted features for self hosted server.

Add Env parameter to allow Register
Make user registration configurable by ENV var
Add SENTRY_ALLOW_REGISTRATION environment variable
add new environment variable to disable registration
Can not disable user registration
How to disable user register on self hosted server ?

Adding the ability to do so to the ENV variables was often discussed and denied. My approach is therefore different: Disable user registration by default.

I guess that disabling user registration is configured in a lot, if not the most sentry setups, as it is a huge security risk NOT to do so.

@dcramer
Copy link
Copy Markdown
Member

dcramer commented May 2, 2017

Unlikely we're going to change this. A few comments:

  1. its not a security risk- most people running onpremise do so because they want it behind the firewall. If you don't want that, you should probably be using sentry.io
  2. if its very common, we could make it configurable in the UI
  3. configuration is part of installation, its not intended that sentry is setup in a way that works for everyone by default

@dcramer dcramer closed this May 2, 2017
@cpoetter
Copy link
Copy Markdown
Author

cpoetter commented May 2, 2017

Thanks @dcramer for the feedback! Allow me to reply to it.

  1. I totally agree with you. It would be really awesome if this could be configured with the help of the UI.
  2. Sure, you can't make it work for everyone, and it should not be our intention to do so. But I still think it's deactivated in most configurations. And isn't a default value meant to be the default in most cases? That it fulfills the need for the majority?

@dcramer
Copy link
Copy Markdown
Member

dcramer commented May 2, 2017

@cpoetter tbh in most cases people use SSO, and this flag doesn't matter at that point

@cpoetter
Copy link
Copy Markdown
Author

cpoetter commented May 2, 2017

@dcramer Okay, you are probably right. But SSO is not really part of this pull request, is it? The question is, in the cases this flag does matter, what value should it have by default?

I also know that I can't claim to know what most people want or do. I only see the amount of pull requests and issues regarding this topic. I think this is at least a hint for something that could be a nice feature/change.

The best would be to make a kind of survey regarding this issue I think.

@dcramer
Copy link
Copy Markdown
Member

dcramer commented May 2, 2017

If we wanted to disable it by default, or make it easier to disable, I'd want to do it via the UI's configuration.

That'd mean:

  • Adding an option to disable registration (vs a feature flag)
    • [Alternatively] Making it so features can be disabled via options (this is more complex and I haven't given it any thought)
  • Exposing the option in setup wizard/settings page
  • Lastly, at this point, we could make new installs disable it by default

@AlexWayfer
Copy link
Copy Markdown

@dcramer I think we should first disable by default, and then do the UI for this, which, of course, is also very important.

Almost 2 months have passed, but I can not find either a PR or an issue about such an option in the UI.

However, last night thousands of people received letters from a script that checked the availability of ALL data on their Sentry servers, because they forgot to turn off the registration, or did not know that it was turned on by default at all. This spawned an issue #5617

I'll summarize my opinion:

  1. First turn off the default registration (it will be safer)
  2. Then make it customizable from the UI

@bumbar1
Copy link
Copy Markdown

bumbar1 commented Jun 24, 2017

We also received mail (luckily from a white hat). There should at least be an option in UI to turn registration off (like gitlab has).

@cpoetter
Copy link
Copy Markdown
Author

Is it possible to reopen this pull request, or should I create a new one?

@dcramer
Copy link
Copy Markdown
Member

dcramer commented Jun 24, 2017

Just to reiterate we are NOT changing the default behavior. The only change we'll accept is exposing this in the UI as a runtime configuration, and as part of the setup wizard. If you want that PR, quite frankly, feel free make it yourself. Sentry is open source and free, but that doesn't mean we'll do whatever you want. Our prioritize is serving our customers and our cloud service, and this is not a priority for either of them.

@AlexWayfer
Copy link
Copy Markdown

Just to reiterate we are NOT changing the default behavior.

Why?

Our prioritize is serving our customers and our cloud service, and this is not a priority for either of them.

How does this relate to the finished change in the default value of a single configuration item, without which thousands of people have problems? You can just press the Merge button and do nothing more.

@dcramer
Copy link
Copy Markdown
Member

dcramer commented Jun 24, 2017

@AlexWayfer those "thousands" of people should learn that internal services need locked down just as much as anything. You're asking for a fairly significant change, with no obvious way to inform users of how to undo it, which will then end up with even more issues here asking how to enable registration, or create accounts, etc. I'm not opposed to making this easier and more obvious to configure, and a choice upon installation, but I am opposed to creating more work for our team 'just because'.

@elafarge
Copy link
Copy Markdown

+1, having Sentry opened to the world by default isn't so safe. Even internally (i.e. behind a VPN/IP filtering) not all employees are supposed to have access to Sentry. I understand you don't want it to trigger more developments on your end, but making this configurable with an environment variable would be pretty easy and would be a bliss for people using your Sentry image on DockerHub.

The default could stay the same but at least, users that want to could disable user registration on container using your official image without having to use a custom sentry.conf.py file.

I can come up with a PR if you're interested.

@dcramer
Copy link
Copy Markdown
Member

dcramer commented Jun 24, 2017

#5620

@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants