Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ACL broken? #226

Closed
manhhavu opened this issue Jul 25, 2019 · 9 comments
Closed

ACL broken? #226

manhhavu opened this issue Jul 25, 2019 · 9 comments

Comments

@manhhavu
Copy link
Contributor

manhhavu commented Jul 25, 2019

Hi,

It seems that ACL for an object is saved incorrectly. Even if I set explicitly an ACL for an object, the object is always considered Public Read + Public Write.

Here is a sample code:

import 'package:parse_server_sdk/parse_server_sdk.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:test_api/test_api.dart';
import 'package:uuid/uuid.dart';

class TestObject extends ParseObject implements ParseCloneable {
  static const String _keyTableName = 'TestObject';
  static const String _keyName = 'Name';

  TestObject() : super(_keyTableName);

  TestObject.clone() : this();

  String get name => get<String>(_keyName);

  set name(String name) => set<String>(_keyName, name);

  @override
  clone(Map map) => TestObject.clone()..fromJson(map);
}

void main() {
  const endpoint = 'https://<parse_endpoint>';

  setUp(() async {
    // Preferences needed by Parse SDK
    SharedPreferences.setMockInitialValues({});

    await Parse().initialize(
      '<client_name>',
      '$endpoint/parse/',
      autoSendSessionId: true,
    );
  });

  test('Create objects for anonymous user', () async {
    await ParseUser.loginWith('anonymous', {'id': Uuid().v4()});
    final user = await ParseUser.currentUser() as ParseUser;
    await user.save();
    expect(user, isNotNull);

    final acl = ParseACL(owner: user);
    acl.setPublicReadAccess(allowed: false);
    acl.setPublicWriteAccess(allowed: false);

    final object1 = TestObject();
    object1.name = 'Belong to user';
    object1.setACL(acl);
    await object1.save();

    final response = await TestObject().getAll();
    final list = response.results.map((el) => el as TestObject);

    expect(list, contains(object1));
  }, timeout: new Timeout(new Duration(minutes: 2)));
}

and its execution's result:

Expected: contains TestObject:<{"className":"TestObject","objectId":"mX75n2D0Xb","createdAt":"2019-07-25T09:42:09.455Z","Name":"Belong to user","ACL":{"yCTHkRGfmO":{"read":true,"write":true}}}>
  Actual: MappedListIterable<ParseObject, TestObject>:[
            TestObject:{"className":"TestObject","objectId":"mX75n2D0Xb","createdAt":"2019-07-25T09:42:09.455Z","updatedAt":"2019-07-25T09:42:09.455Z","Name":"Belong to user"}
          ]

you can see the one returned by the query does not contains ACL at all.

It looks like a lot the problem mentioned in issue #129 but I supposed it was fixed. So it must be something else then?

Thanks,

@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Jul 26, 2019

Hello @manhhavu
The result conforms to what you defined in the code.
You created an object, set the owner, and set that the Public cannot read or write. So only the owner has access to the object.

    final acl = ParseACL(owner: user);
    acl.setPublicReadAccess(allowed: false);
    acl.setPublicWriteAccess(allowed: false);

The ACL result will contain only the read and write Owner id.

"ACL":{"yCTHkRGfmO":{"read":true,"write":true}}

@manhhavu
Copy link
Contributor Author

Hi @RodrigoSMarques ,

Thanks for your quick reply. Actually, I might not be very clear in my initial question (I've corrected it a little bit). I've rewritten the initial test into two scenarios to be more precise about the issue (at least I hope so).

  • 1st scenario. Create object with restricted ACL, query the same object from ACL, check if this object' ACL is the same as the initially created one. The scenario is failed. The object from server' ACL is not restricted for the user who created it.
  • 2nd scenario. Create object with restricted ACL for user1. Sign that user1 out. user2 #, he should not be able to see object of user1, but he actually sees it. It is because the object was saved as Public Read + Public Write but not the restricted ACL.

Please tell me if I missed something. There are the code of two tests, the screenshot of Parse Dashboard and the tests' execution results below:

import 'package:parse_server_sdk/parse_server_sdk.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:test_api/test_api.dart';
import 'package:uuid/uuid.dart';

class TestObject extends ParseObject implements ParseCloneable {
  static const String _keyTableName = 'TestObject';
  static const String _keyName = 'Name';

  TestObject() : super(_keyTableName);

  TestObject.clone() : this();

  String get name => get<String>(_keyName);

  set name(String name) => set<String>(_keyName, name);

  @override
  clone(Map map) => TestObject.clone()..fromJson(map);
}

void main() {
  const endpoint = 'https://<parse_endpoint>';

  setUp(() async {
    // Preferences needed by Parse SDK
    SharedPreferences.setMockInitialValues({});

    await Parse().initialize(
      '<client_name>',
      '$endpoint/parse/',
      autoSendSessionId: true,
    );
  });

  test('ACL should limit access for objects', () async {
    await ParseUser.loginWith('anonymous', {'id': Uuid().v4()});
    final user = await ParseUser.currentUser() as ParseUser;
    await user.save();
    expect(user, isNotNull);

    final acl = ParseACL(owner: user);
    acl.setPublicReadAccess(allowed: false);
    acl.setPublicWriteAccess(allowed: false);

    TestObject object1 = TestObject();
    object1.name = 'Belong to user';
    object1.setACL(acl);
    await object1.save();

    expect(object1.getACL().getPublicReadAccess(), isFalse);
    expect(object1.getACL().getPublicWriteAccess(), isFalse);

    // Same object1 retrieved from server
    TestObject object1FromServer = (await retrieveAll()).first;
    // Check if the same object
    expect(object1.objectId, equals(object1FromServer.objectId));

    print('ObjectID = ${object1.objectId}');

    // Even these assertions pass but in the Parse Dashboard, they are
    // 'Public Read + Public Write', screenshot is below
    expect(object1FromServer.getACL().getPublicReadAccess(), isFalse);
    expect(object1FromServer.getACL().getPublicWriteAccess(), isFalse);
    // Suprisingly, these assertions failed.
    expect(
        object1FromServer.getACL().getReadAccess(userId: user.objectId), isTrue,
        reason: "user should be able to read object1");
    expect(object1FromServer.getACL().getWriteAccess(userId: user.objectId),
        isTrue,
        reason: "user should be able to write object1");
  }, timeout: new Timeout(new Duration(minutes: 2)));

  test(
      'Object limited with a not public access ACL should not be seen by other user',
      () async {
    // User1 #
    await ParseUser.loginWith('anonymous', {'id': Uuid().v4()});
    final user1 = await ParseUser.currentUser() as ParseUser;
    await user1.save();

    final acl = ParseACL(owner: user1);
    acl.setPublicReadAccess(allowed: false);
    acl.setPublicWriteAccess(allowed: false);

    TestObject object1 = TestObject();
    object1.name = 'Belong to user';
    object1.setACL(acl);
    await object1.save();

    expect(object1.getACL().getPublicReadAccess(), isFalse);
    expect(object1.getACL().getPublicWriteAccess(), isFalse);

    // User1 sign out
    user1.logout(deleteLocalUserData: true);

    // User 2 #
    await ParseUser.loginWith('anonymous', {'id': Uuid().v4()});
    final user2 = await ParseUser.currentUser() as ParseUser;
    await user2.save();

    expect(user1.objectId != user2.objectId, isTrue);

    // Retrieve objects seen by user2
    final list = await retrieveAll();
    // Make sure user2's objects does not contain object 1
    for (var o in list) {
      expect(o.objectId != object1.objectId, isTrue,
          reason: "User2 should not be able see object1");
    }
  }, timeout: new Timeout(new Duration(minutes: 2)));
}

Future<List<TestObject>> retrieveAll() async {
  final response = await TestObject().getAll();
  return response.results.map((el) => el as TestObject).toList();
}

Parse Dashboard screenshot, object1's ACL is Public Read+Public Write
parse-dashboard

First test execution

ObjectID = cOJc9L3EEn
package:test_api                       expect
test/not_working_parse_test.dart 66:5  main.<fn>
===== asynchronous gap ===========================
dart:async                             _AsyncAwaitCompleter.completeError
test/not_working_parse_test.dart       main.<fn>
===== asynchronous gap ===========================
dart:async                             _asyncThenWrapperHelper
test/not_working_parse_test.dart       main.<fn>

Expected: true
  Actual: <false>
user should be able to read object1

Second test execution

package:test_api                        expect
test/not_working_parse_test.dart 108:7  main.<fn>
===== asynchronous gap ===========================
dart:async                              _AsyncAwaitCompleter.completeError
test/not_working_parse_test.dart        main.<fn>
===== asynchronous gap ===========================
dart:async                              _asyncThenWrapperHelper
test/not_working_parse_test.dart        main.<fn>

Expected: true
  Actual: <false>
User2 should not be able see object1

@RodrigoSMarques
Copy link
Contributor

what version of lib are you using?

@manhhavu
Copy link
Contributor Author

I'm using the lib's version 1.0.22 in Flutter v1.7.8+hotfix.3

@RodrigoSMarques
Copy link
Contributor

Hi @manhhavu
I am using 1.0.21 and I have no problems.
Version 1.0.22 has had major changes to the save method and I don't know if this could have impacted the ACL definition.
I could not use this version because my application does not work with it and as I am in a project I was unable to verify.

Do the test in version 1.0.21 and report the result.
In pubsepec.yaml do the following.

  parse_server_sdk: '1.0.21'

@manhhavu
Copy link
Contributor Author

manhhavu commented Jul 29, 2019

Re @RodrigoSMarques ,

I've tested the 1.0.21 as you suggested but I always have the same problem. It seems that the ACL is not included in the request being sent to the server.

I've activated the debug mode the SDK and here the log:

flutter: ╭-- Parse Request
flutter: curl -X POST -H 'content-type: text/plain; charset=utf-8' -H 'user-agent: Flutter Parse SDK 1.0.22' -H 'X-Parse-Application-Id: moove' -H 'X-Parse-Session-Token: r:85547558644e4b8b37011ffbe6e0f5e3' -d '{"Name":"Belong to user"}' https://<server>/parse/classes/TestObject

 https://<server>/parse/classes/TestObject
flutter: ╰--
flutter: ╭-- Parse Response
Class: TestObject
Function: ParseApiRQ.create
Status Code: 201
Payload: {"className":"TestObject","objectId":"pHyyZkmD9g","createdAt":"2019-07-29T16:14:08.650Z","Name":"Belong to user","ACL":{"9dGL5fT6La":{"read":true,"write":true}}}
╰--

You can see that the request payload does not include at all ACL section. (Curious enough, the response log includes ACL which does not make sense. I think it just the returns the initial object.)

You said that you have no problem with this version 1.0.21. Do you use ACL in your project and do you have any particular configuration?

Thanks,

@manhhavu
Copy link
Contributor Author

I think that I might find the bug:

  1. When a ParseObject.save() is called, this method calls (either via create() or update()) the method toJson(forApiRQ: true)
  2. In the method toJson(forApiRQ), there is this line:
final Map<String, dynamic> target = forApiRQ
        ? _unsavedChanges
        : _getObjectData();

So, when forApiRQ = true, target = _unsavedChanges.
3. But when we set ACL for an object, this is the code of Parse Base:

void setACL<ParseACL>(ParseACL acl) {
    _getObjectData()[keyVarAcl] = acl;
}

We can see that only the map in _getObjectData() is changed but the ACL is never added to _unsavedChanges, so it will never get sent to the server, then never be saved.
4. The correct code for this implementation should be

void setACL<ParseACL>(ParseACL acl) {
    // The set method updates _getObjectData() 
    // and the _unsavedChanges at the same time
    set(keyVarAcl, acl); 
}

I've tested this code snippet and it actually passed my unit tests above.

Can you confirm my hypothesis above? If it is the case, I'd not mind to create a PR. 😃

manhhavu added a commit to moovetech/flutter_parse_sdk that referenced this issue Jul 30, 2019
@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Aug 1, 2019

Hi @manhhavu.
You're right. The ACL has problems with version 1.0.22, but 1.0.21 is OK for me.
My code:

      ParseACL parseACL = ParseACL(owner: currentUser);
      parseACL.setPublicReadAccess(allowed: true);
      parseACL.setPublicWriteAccess(allowed: false);

Result:
Captura de Tela 2019-08-01 às 19 57 13

I did the test with your change and it is working. You can send the PR to branch 1.0.23

manhhavu added a commit to moovetech/flutter_parse_sdk that referenced this issue Aug 2, 2019
@manhhavu
Copy link
Contributor Author

manhhavu commented Aug 2, 2019

Hi @RodrigoSMarques , PR sent 😃

phillwiggins pushed a commit that referenced this issue Aug 2, 2019
@manhhavu manhhavu closed this as completed Aug 2, 2019
fischerscode pushed a commit to fischerscode/Parse-SDK-Flutter that referenced this issue Sep 5, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants