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

user can't update its own data (401) #125

Open
Morriz opened this issue Jan 31, 2015 · 17 comments
Open

user can't update its own data (401) #125

Morriz opened this issue Jan 31, 2015 · 17 comments

Comments

@Morriz
Copy link

Morriz commented Jan 31, 2015

The loopback angular model.$save (or Model.update for that matter) defaults to $upsert, which tries a batch update on the server, which is not allowed for just the $owner. The PUT method for a logged in user in the explorer works though.

Difference in calls is (while using same body payload):

explorer PUT: /api/Users/someid (allowed)
loopback-services.js PUT: /api/Users?id=someid (not allowed because of reason mentioned)

output after call from loopback-services.js:

loopback:security:role isInRole(): $everyone +0ms
  loopback:security:access-context ---AccessContext--- +1ms
  loopback:security:access-context principals: +0ms
  loopback:security:access-context principal: {"type":"USER","id":1} +0ms
  loopback:security:access-context modelName User +0ms
  loopback:security:access-context modelId 1 +0ms
  loopback:security:access-context property upsert +0ms
  loopback:security:access-context method upsert +0ms
  loopback:security:access-context accessType WRITE +0ms
  loopback:security:access-context accessToken: +0ms
  loopback:security:access-context   id "KCXtsjtfaDpwMh2IExPFHCS3DgIAF1oL56hW7mWCbCvVoFcExbzJAWW6DAkVSQAZ" +0ms
  loopback:security:access-context   ttl 1209600 +0ms
  loopback:security:access-context getUserId() 1 +1ms
  loopback:security:access-context isAuthenticated() true +0ms
  loopback:security:role Custom resolver found for role $everyone +0ms
  loopback:security:acl The following ACLs were searched:  +0ms
  loopback:security:acl ---ACL--- +0ms
  loopback:security:acl model User +0ms
  loopback:security:acl property * +0ms
  loopback:security:acl principalType ROLE +0ms
  loopback:security:acl principalId $everyone +0ms
  loopback:security:acl accessType * +0ms
  loopback:security:acl permission DENY +0ms
  loopback:security:acl with score: +0ms 7495
  loopback:security:acl ---Resolved--- +0ms
  loopback:security:access-context ---AccessRequest--- +0ms
  loopback:security:access-context  model User +1ms
  loopback:security:access-context  property upsert +0ms
  loopback:security:access-context  accessType WRITE +0ms
  loopback:security:access-context  permission DENY +0ms
  loopback:security:access-context  isWildcard() false +0ms
  loopback:security:access-context  isAllowed() false +0ms
@joncarlson
Copy link

Also experiencing this (not using Postgres). If I change the URL in $upsert from urlBase + '/people' to urlBase + '/people/:id' updates work correctly.

@bajtos
Copy link
Member

bajtos commented Mar 4, 2015

IIRC, there is an important difference between "PUT /api/people" and "PUT /api/people/:id": the former replaces all properties with the data in the request and deletes any properties not included in the request, while the latter updates only the properties specified by the request and leaves the rest untouched.

@raymondfeng @ritch you are more familiar with ACLs than I am. Is it expected that updateOrCreate is not allowed for $owner role?

@bajtos
Copy link
Member

bajtos commented Mar 4, 2015

Apparently PUT /api/people (updateOrCreate) and PUT /api/people/:id (prototype.updateAttributes) is all that loopback provides, in which case we have to use "updateAttributes" to update the current user.

There are two things that can be done.

A) Change your Angular app to call updateAttributes instead of save:

// before: User.$save();
User.prototype$updateAttributes({ id: user.id }, user);

B) Rework the implementation of $save (source) to detect whether the id property is set. If it is, then call Model.prototype$updateAttributes, otherwise call Model.create.

@nolandubeau
Copy link

I would vote for B.

On Wednesday, March 4, 2015, Miroslav Bajtoš [email protected]
wrote:

Apparently PUT /api/people (updateOrCreate) and PUT /api/people/:id
(prototype.updateAttributes) is all that loopback provides, in which case
we have to use "updateAttributes" to update the current user.

There are two things that can be done.

A) Change your Angular app to call updateAttributes instead of save:

// before: User.$save();
User.prototype$updateAttributes({ id: user.id }, user);

B) Rework the implementation of $save (source

// Angular always calls POST on $save()
// This hack is based on
// http://kirkbushell.me/angular-js-using-ng-resource-in-a-more-restful-manner/
resource.prototype.$save = function(success, error) {
// Fortunately, LoopBack provides a convenient `upsert` method
// that exactly fits our needs.
var result = resource.upsert.call(this, {}, this, success, error);
return result.$promise || result;
};
)
to detect whether the id property is set. If it is, then call
Model.prototype$updateAttributes, otherwise call Model.create.


Reply to this email directly or view it on GitHub
#125 (comment)
.

Nolan Dubeau
VP, Engineering
Guardly

Sent from my Apple Newton

@bajtos
Copy link
Member

bajtos commented Mar 5, 2015

As I mentioned in loopbackio/loopback-datasource-juggler#481, another option is to add a new method "prototype.update" that would always update all attributes and which can be used in $save.

@pbrain19
Copy link

this issue does not seem to be resolved. Currently trying to update customer and it does not seem to have the ownership to do so.

@rhoderunner9
Copy link

The documentation online does not seem to be sufficient to cover how to do simple tasks. I have set up the updateAttributes, and even just threw my hands up and gave $owner unlimited access, and still get the Authentication Required error. This tells me that this user is not the owner of his/her data. The documentation is vast, yet seems to leave many of us hanging with inadequate answers to very common issues. I have gone so far as to set the "property": "update" to ALLOW for $everyone, yet I still get Authentication Required. I'm assuming I'm one of thousands of people spending amazing amounts of human hours on these kind of issues, so could someone clarify how to setup the model update ACL effectively?

@superkhau
Copy link
Contributor

@crandmck Maybe you can help out with the docs here? @rhoderunner9 Do you have any suggestions on how to improve it?

@Sequoia
Copy link
Collaborator

Sequoia commented Sep 27, 2016

@superkhau is there a bullet list/nutshell version of what the proper solution is here? I can write documentation with examples etc. once I know what the recommended approach is but it's not clear from this thread what it is.

Can @bajtos or someone else familiar with the topic clarify the best/proper way to:

  1. set up ACLs to work with the angular SDK
  2. call the API via the angular SDK ($save, $update...?)

Alternately, I can dig in & figure out the solution myself but that is likely to be very time consuming & probably wasted/duplicated effort.

@superkhau
Copy link
Contributor

@bajtos Would know more about the design/intent of the Angular SDK here.
@raymondfeng Could help with the intent of the ACL parts.

With regards to 1 and 2, https://github.com/strongloop/loopback-getting-started-intermediate has a flushed out example that uses Angular which ACLs to demonstrate the typical use case. The issue is either a (the workaround) or b (the feature request) from @bajtos's response.

@bajtos bajtos added the #tob label Oct 5, 2016
@bajtos
Copy link
Member

bajtos commented Oct 5, 2016

I am afraid I don't have enough bandwidth to look into this issue right now. I'm moving it to #tob so that we can set aside time to work on this as part of our Scrum process.

@Sequoia Sequoia removed their assignment Oct 5, 2016
@Sequoia
Copy link
Collaborator

Sequoia commented Oct 5, 2016

Please re-assign me when this is ready-for-docs! 🌴

@crandmck
Copy link
Contributor

@bajtos bump....?

@bajtos
Copy link
Member

bajtos commented Nov 16, 2016

@davidcheung do you perhaps have bandwidth to help me with this issue? I think we should be able to reproduce this problem in our test suite (enable authentication, create a user, try to update the user via userInstance.$save()). I can investigate myself what's going on once we have the new test added (e.g. in a feature branch).

@davidcheung
Copy link
Contributor

yeah this is still a problem, will add to my backlog
can reproduce via : https://github.com/strongloop/loopback-sdk-angular/compare/user-cannot-save

@luncht1me
Copy link

luncht1me commented Feb 1, 2017

This is still a problem...

upsert() is doing a PATCH to Model?id= instead of Model/id and failing miserably.

I don't like having to modify my lb-services each time we generate it on an update as per @joncarlson's suggestion.... It's so elegant to just do a Model.upsert(instance) and have it update properly... Having to do Model.update({where:{id:instance.id}}, _.omit(instance, 'id')) is a real pain in the arse when upsert worked in previous versions.

Dangerous things happen when using Update, and not validating that id is actually defined. One of our juniors overwrote everything in a collection by doing this w/o a properly verified id....

@rhoderunner9
Copy link

I have run into this issue several times. I think one possible solution is to modify the service generation app that writes the normal interfaces to the same sdk directory, making all the generated classes abstract. A secondary directory (something like "sdk-impl") can be generated if it does not exist, otherwise providing feedback that an interface has changed, and therefore should be verified so that individual modifications are maintained. If the interface to the service hasn't changed, no notification is necessary. If it has changed, then a notification can be specific to the changes. I don't think this would be too difficult. A simple properties file in the pre-existing sdk specifying the previous api could be used to provide information on the changes that need to be addressed in the sdk-impl. I've been maintaining my modifications in a WICHTIG.md file that keeps track of the things that need to be changed each time I generate the sdk, so it'd be nice to rely on my change set.

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

No branches or pull requests