----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3544/#review4769 -----------------------------------------------------------
See inline comments. http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java <https://reviews.apache.org/r/3544/#comment10491> Is the intent here to punish someone who is trying to update someone else's data? :) Scenario: Viewer = john.doe POST to rpc endpoint [ { "method":"people.update", "id":"people.update", "params":{ "person":{ "displayName":"Foo Bar" }, "userId":"jane.doe" } } ] POST to rpc endpoint [ { "method":"people.get", "id":"people.get", "params":{ "fields":[ "name", "displayName" ], "userId":"@viewer", "groupId":"@self" } } ] Result: John Doe's display name is now "Foo Bar"! My suggestion is that you create a protected method in PersonServiceDb to check if a person is allowed to update the given person record. By default this method would return true iff the viewer and person id are the same. If the method returned false, we would want to return a 403 or the like. Someone could then override this to provide the admin functionality we've discussed previously. - Stanton On 2012-02-02 10:53:21, Evgeny Bogdanov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3544/ > ----------------------------------------------------------- > > (Updated 2012-02-02 10:53:21) > > > Review request for shindig. > > > Summary > ------- > > A patch that enables API for updating person data > PUT people/12/@self > osapi.people.update() > > > for this JIRA reports > https://issues.apache.org/jira/browse/SHINDIG-1674 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java > 1239531 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java > 1239531 > > Diff: https://reviews.apache.org/r/3544/diff > > > Testing > ------- > > tests are in the patch > > > Thanks, > > Evgeny > >
