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 API endpoint to create a user. #1542

Merged
merged 1 commit into from Oct 7, 2015

Conversation

Projects
None yet
5 participants
@ketan
Copy link
Member

commented Oct 6, 2015

The contract is same as that of the PATCH API call, with an
additional REQUIRED FIELD login_name.

@ketan

This comment has been minimized.

Copy link
Member Author

commented Oct 6, 2015

I'll submit a PR for the api docs when this is merged.

@ketan ketan added this to the Release 15.3 milestone Oct 6, 2015

def create
@user_to_operate = user_service.findUserByName(params[:login_name])

unless @user_to_operate.instance_of?(com.thoughtworks.go.domain.NullUser)

This comment has been minimized.

Copy link
@jyotisingh

jyotisingh Oct 6, 2015

Contributor

Checking for existence of user over here is fine, but there could possibly be a race condition. UserService.update internally calls saveOrUpdate which does not check if the user already exists. Have a look at addUserIfDoesNotExist instead.

This comment has been minimized.

Copy link
@ketan

ketan Oct 6, 2015

Author Member

I'm aware of this problem with possible race condition. However addUserIfDoesNotExist is not what I'm looking to do – I want to raise an error if a user already exists, or create if none exists, unless you mean to use enableUserMutex to acquire a lock.

This comment has been minimized.

Copy link
@jyotisingh

jyotisingh Oct 7, 2015

Contributor

I din't mean to say use addUserIfDoesNotExist, but use enableUserMutex as used in this method to avoid race-conditions.

@ketan ketan force-pushed the ketan:user-create branch 2 times, most recently from 1507c7f to c4499e3 Oct 7, 2015

@ketan

This comment has been minimized.

Copy link
Member Author

commented Oct 7, 2015

@jyotisingh - please review again.

@ketan ketan force-pushed the ketan:user-create branch 3 times, most recently from aeeff70 to 6afa239 Oct 7, 2015

Add API endpoint to create a user.
The contract is same as that of the `PATCH` API call, with an
additional REQUIRED FIELD `login_name`.

@ketan ketan force-pushed the ketan:user-create branch from 6afa239 to 77d321c Oct 7, 2015

jyotisingh added a commit that referenced this pull request Oct 7, 2015

Merge pull request #1542 from ketan/user-create
Add API endpoint to create a user.

@jyotisingh jyotisingh merged commit 5efbeae into gocd:master Oct 7, 2015

5 checks passed

build-linux-PR/build-non-server
Details
build-linux-PR/build-server
Details
build-linux-PR/check_tlb_correctness
Details
build-linux-PR/create-maven-release
Details
build-linux-PR/go-sdk
Details

@ketan ketan deleted the ketan:user-create branch Oct 7, 2015

def create
result = HttpLocalizedOperationResult.new

user, created = user_service.withEnableUserMutex do

This comment has been minimized.

Copy link
@arvindsv

arvindsv Oct 7, 2015

Member

Shouldn't this whole block really be in UserService?

This comment has been minimized.

Copy link
@ketan

ketan Oct 7, 2015

Author Member

As much as I'd love for it to be, the method save from the controller cannot be moved into java without writing about 100 lines of java code :(

This comment has been minimized.

Copy link
@arvindsv

arvindsv Oct 7, 2015

Member

Because of tristate? Anyway, I'm sure you've thought about it more than I have. My question was based on how a user is created, when called from the admin page.

This comment has been minimized.

Copy link
@ketan

ketan Oct 7, 2015

Author Member

Hmm, the create method on UserService is interesting, I think I should be able to use it.

@ketan

This comment has been minimized.

Copy link
Member Author

commented on server/src/com/thoughtworks/go/server/service/UserService.java in 77d321c Oct 8, 2015

@arvindsv @jyotisingh - do you know why this condition for PASSWORD_FILE exists? I'm trying to understand and simplify the various validations and conditions to create a user all over this class.

This comment has been minimized.

Copy link
Member

replied Oct 9, 2015

I don't know, but it seems to say "If the user doesn't come from a password file (meaning, comes from LDAP), validate both email and matcher". Could it be because users from a password file don't have an email address set, unlike LDAP users?

@zabil zabil removed this from the Release 15.3 milestone Nov 19, 2015

@ketan ketan added this to the Release 15.3 milestone Nov 24, 2015

@patrick92100

This comment has been minimized.

Copy link

commented Jan 7, 2016

I did not find any details on how to create user in the API doc (https://api.go.cd/current/#users)? did I miss something?
Regards

@ketan

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2016

@patrick92100 users are created when they first login, therefore there is no way to create users.

@arvindsv

This comment has been minimized.

Copy link
Member

commented Jan 7, 2016

If you need to add them to roles, you can do it in the config. Even if they don't exist (not created), I believe.

@patrick92100

This comment has been minimized.

Copy link

commented Jan 7, 2016

ok, I am a bit confusing, because I saw in the 15.3 change log "Add API endpoint to create a user." but in the 15.2, user was also created at the first login...

@ketan

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2016

Hmm... I'm mistaken, the user create endpoint actually exists. It was never documented, because we forgot to do it.

If you POST to the endpoint go/api/users with the user attributes in JSON, you should be OK. You will see the attributes in the update user api docs, you will need to submit an additional login name parameter.

I'll document the endpoint later tomorrow.

@ketan

This comment has been minimized.

Copy link
Member Author

commented Jan 8, 2016

@patrick92100 — the new api docs are published - https://api.go.cd/current/#create-a-user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.