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 REST API for creating an account #9572

Merged
merged 5 commits into from Dec 24, 2018

Conversation

Projects
None yet
4 participants
@Gargron
Copy link
Member

Gargron commented Dec 19, 2018

The method is available to apps with a token obtained via the client credentials grant. It creates a user and account records, as well as an access token for the app that initiated the request. The user is
unconfirmed, and an e-mail is sent as usual.

The method returns the access token, which the app should save for later. The REST API is not available to users with unconfirmed accounts, so the app must be smart to wait for the user to click a
link in their e-mail inbox.

The method is rate-limited by IP to 5 requests per 30 minutes.

Motivation: Multiple app developers have requested this. Currently an end-user cannot just install a mobile app and start using Mastodon, they must visit the website first. This, on the other hand, would allow a fully immersive sign-up process. The mobile app can implement a server picker, etc...

Risks: The API can be used to create spam bots. However, it is just as easy for a dedicated spammer to use the HTML form to create spam bots, and this API method actually has a higher rate limit (5/30 min here vs 25/30 min). E-mail confirmation is still required, so nothing changes there...

@Gargron Gargron added the api label Dec 19, 2018

@@ -68,7 +68,7 @@ def current_user
end

def require_user!
if current_user && !current_user.disabled?
if current_user && !current_user.disabled? && current_user.confirmed?

This comment has been minimized.

@Gargron

Gargron Dec 19, 2018

Author Member

This condition was unneccessary before because unconfirmed users had no way to obtain an access token.

@@ -16,6 +17,16 @@ def show
render json: @account, serializer: REST::AccountSerializer
end

def create
token = AppSignUpService.new.call(doorkeeper_token.application, account_params)
response = Doorkeeper::OAuth::TokenResponse.new(token)

This comment has been minimized.

@Gargron

Gargron Dec 19, 2018

Author Member

This mirrors the usage of this class by Doorkeeper controllers.

@nolanlawson

This comment has been minimized.

Copy link
Collaborator

nolanlawson commented Dec 19, 2018

This looks great. The only downside I can foresee is that (as you pointed out), the username and password are exposed to the app. But I don't see any way around that.

Having the app show a temporary screen saying "please go click a link in your inbox" is fine, but it's a bit odd that that link will go to the main Mastodon frontend, and there's no way for the app to intercept it or at least be notified that the user is now verified.

Maybe the API could accept a redirect URL as well, and the Mastodon frontend could display some text like, "Thanks for confirming your email. Application FooApp would like you to continue your signup here."

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Dec 19, 2018

Maybe the API could accept a redirect URL as well, and the Mastodon frontend could display some text like, "Thanks for confirming your email. Application FooApp would like you to continue your signup here."

Hm... So far it was possible to implement this PR without changing data structures or requiring apps to re-register, but I don't think an extra redirect could be made with existing parts... Need to save information that a user was created by an app to be able to do that... Plus add an extra redirect URI to apps... Unless it would be OK to redirect users to the already saved OAuth redirect URI...

@nolanlawson

This comment has been minimized.

Copy link
Collaborator

nolanlawson commented Dec 19, 2018

Hmmm, redirecting to the already saved OAuth URL would probably be fine. The app can just poke the API at verify_credentials to confirm that the user is confirmed, so no other info is required.

@Gargron Gargron force-pushed the feature-app-sign-up branch from 8245896 to 322869a Dec 20, 2018

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Dec 20, 2018

Done

@mayaeh

This comment has been minimized.

Copy link
Collaborator

mayaeh commented Dec 20, 2018

I think that we need to check the rules and terms and the conditions of it instance before creating an account. What do you think?

Also, how about making it possible to switch by setting ON/OFF of this API?

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Dec 20, 2018

I think that we need to check the rules and terms and the conditions of it instance before creating an account. What do you think?

An app could implement a web view to /about/more. Since rules are HTML, with potentially complex things, images, iframes, you would end up needing to render a web page anyway.

Also, how about making it possible to switch by setting ON/OFF of this API?

Maybe. Apps must already deal with this API missing from older versions, so there is no consistency to lose.

@mayaeh

This comment has been minimized.

Copy link
Collaborator

mayaeh commented Dec 20, 2018

An app could implement a web view to /about/more. Since rules are HTML, with potentially complex things, images, iframes, you would end up needing to render a web page anyway.

Fediverse have also instances for specific topics.
It is unfortunate that a mismatch occurs that "I created an account from an application, but it was not a place to accept me".

I think that it is heavy to let app developers handle all the measures to prevent them.

This is one idea:

  1. When requesting creation of an account with this API, the server first returns a link (/about etc.) and a unique ID.
  2. Then, the account registration is accepted only within a certain period after receiving the flag of "agree" and its ID with this API.

(I think that it is necessary to delete the registration unique ID expired periodically by cron job etc.)

I think it is in vain to send it link (/about, etc.) because it is already known.
But I hope they will read it before registering.

@mayaeh

This comment has been minimized.

Copy link
Collaborator

mayaeh commented Dec 20, 2018

Maybe. Apps must already deal with this API missing from older versions, so there is no consistency to lose.

Several servers seem to be adding reCAPTCHA to account registration of WebUI.
If we merge this API in this state, I think that the effect of reCAPTCHA will be lost.
Several server administrators are afraid that this will again increase spam accounts.

Therefore, it seems that they want an option to disable this API.

(I do not introduce reCAPTCHA, because that is difficult for me to break through.)

Gargron added some commits Dec 19, 2018

Add REST API for creating an account
The method is available to apps with a token obtained via the client
credentials grant. It creates a user and account records, as well as
an access token for the app that initiated the request. The user is
unconfirmed, and an e-mail is sent as usual.

The method returns the access token, which the app should save for
later. The REST API is not available to users with unconfirmed
accounts, so the app must be smart to wait for the user to click a
link in their e-mail inbox.

The method is rate-limited by IP to 5 requests per 30 minutes.

@Gargron Gargron force-pushed the feature-app-sign-up branch from ddb6e5a to 232e3dc Dec 22, 2018

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Dec 23, 2018

When requesting creation of an account with this API, the server first returns a link (/about etc.) and a unique ID.

There is no guarantee that an app will show anything to the user, it could just send the ID back straight away. Likewise, even without the ID an app could make the user read through that page and click "Agree"

Also, how about making it possible to switch by setting ON/OFF of this API?

I have been thinking about the on/off switch for this feature, and I think the admin settings UI has become too complex. Everything is in one form, that doesn't work well. I wouldn't want to add one more checkbox to it. :(

@Neustrashimy

This comment has been minimized.

Copy link

Neustrashimy commented Dec 24, 2018

Could you implement Boolean "confirmed to terms" parameter for creation API?
I want to get confirm that new user has read my instance's custom terms/rules and agreed.
thanks.

@mayaeh

This comment has been minimized.

Copy link
Collaborator

mayaeh commented Dec 24, 2018

There is no guarantee that an app will show anything to the user, it could just send the ID back straight away. Likewise, even without the ID an app could make the user read through that page and click "Agree"

I agree with that.
Besides, current flow of account creation API is consistent with WebUI.

However, WebUI has the following notation:
By clicking "Sign up" below you agree to follow the rules of the instance and our terms of service.

I think that if there is no equivalent to this API, if a registered user causes trouble, it will be considered "This response is unjust because you are not showing such a document".

I think that this complaint will be directed to the instance administrator, not to the application developer.

I have been thinking about the on/off switch for this feature, and I think the admin settings UI has become too complex. Everything is in one form, that doesn't work well. I wouldn't want to add one more checkbox to it. :(

I think that there are really many items...

@Gargron Gargron force-pushed the feature-app-sign-up branch from b88d7d3 to 2c09d30 Dec 24, 2018

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Dec 24, 2018

Okay, I have added required agreement param to the API.

@mayaeh

This comment has been minimized.

Copy link
Collaborator

mayaeh commented Dec 24, 2018

Thank you for considering!

@Neustrashimy

This comment has been minimized.

Copy link

Neustrashimy commented Dec 24, 2018

thanks a lot!

@Gargron Gargron force-pushed the feature-app-sign-up branch from 2c09d30 to fa5f165 Dec 24, 2018

@Gargron Gargron force-pushed the feature-app-sign-up branch from fa5f165 to 2bf7bb5 Dec 24, 2018

@mayaeh

This comment has been minimized.

Copy link
Collaborator

mayaeh commented Dec 24, 2018

I confirmed that I can create an account using this API.
There is no error as far as I can see.

I think that it is necessary to carefully write a document, such as handling "agreement" flag later.

@mayaeh

mayaeh approved these changes Dec 24, 2018

@Gargron Gargron merged commit 5d2fc6d into master Dec 24, 2018

11 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.3 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.3 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate Approved by Eugen.
Details

@Gargron Gargron deleted the feature-app-sign-up branch Dec 24, 2018

@neet neet referenced this pull request Jan 20, 2019

Merged

WIP: Support Mastodon 2.7.0 #36

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment