Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Set system.creator based on Auth on Create WI REST Endpoints #244

Closed
aslakknutsen opened this issue Sep 13, 2016 · 8 comments
Closed

Set system.creator based on Auth on Create WI REST Endpoints #244

aslakknutsen opened this issue Sep 13, 2016 · 8 comments
Milestone

Comments

@aslakknutsen
Copy link
Contributor

aslakknutsen commented Sep 13, 2016

  • Fail or Ignore? set system.creator on Update

Depends on #229

@pranavgore09
Copy link
Contributor

pranavgore09 commented Oct 7, 2016

@aslakknutsen @tsmaeder @baijum : please can you verify this flow for solving the task :-

  • Create a new UserType
type UserType struct {
    SimpleType
    Value string // <- user email ?
}
  • Update (f *FieldDefinition) UnmarshalJSON to have a case KindUser:
  • Add convertToModel method to the UserType. (this is more about validation and not exactly conversion, any other place to put this validation?)
accepts user email and validates against DB
  • Add convertFromModel method to the UserType.
Take user email from DB, find out identity and return the identity instance or user instance ?
  • Write a middleware which will insert current logged in user's email in the context
  • WIrepo.create will set context.currentUser to fields["system.creator"]

Assumptions

  • Field type "system.creator" is present on WI and has Kind="User"
    This assumption is based on current codebase of migration.go

Questions

  • What should be passed in the field system.creator by API client ? IMO, it should be done my alm backend. Currently we have that field as required:true. Should that be changed ?
  • Is saving email in system.creator sufficient ? or it should save user JSON ?

@baijum
Copy link
Contributor

baijum commented Oct 7, 2016

Using a middleware to extract the user email and setting it to context looks like a good idea. Maybe you can take a look at this implementation for some inspiration: https://github.com/auth0/go-jwt-middleware

@aslakknutsen
Copy link
Contributor Author

@aslakknutsen
Copy link
Contributor Author

What should be passed in the field system.creator by API client ? IMO, it should be done my alm backend.

The backend will populate and override whatever the client says here based on the provided AuthToken

Currently we have that field as required:true. Should that be changed ?

Hmm, good question. It's not required by Client to input, but it's required for WI to be 'complete'. We're currently not really separating on that. @kwk @tsmaeder thoughts?

@pranavgore09
Copy link
Contributor

@aslakknutsen thanks, I did not see TokenManager.
@baijum thank you. i checked go-jwt-middleware, but as of now I am thinking of wiring up Locate method in first part. and then can move it to a middlware.

@pranavgore09
Copy link
Contributor

reference for testing "logged in" user. https://gist.github.com/aslakknutsen/d11944c924acfd32bedb60ebbf9a8d7f

@aslakknutsen aslakknutsen modified the milestones: Sprint #121, Sprint #122 Oct 19, 2016
@pranavgore09
Copy link
Contributor

@aslakknutsen @tsmaeder , currently we are setting syste,creator in POST call.
Should we have something like modified_by in WI.fields in case of PUT, PATCH requests ?
If yes, can I add that field via migration ?

(Not to be done in this sprint or in this PR/issue just asking here because of relevance)

@aslakknutsen
Copy link
Contributor Author

Should we have something like modified_by in WI.fields in case of PUT, PATCH requests ? If yes, can I add that field via migration ?

Yes, but it doesn't really make sense until we track changes individually:
#313

Else we would only store who changed it last time. I would hold on of adding any modified_by WI field until we know how we're implementing it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants