New issue
Advanced search Search tips

Issue 664441 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
V1



Sign in to add a comment

authpolicy: Add unit test for SambaInterface

Project Member Reported by ljusten@chromium.org, Nov 11 2016

Issue description

Brain storm:
Actual Samba commands won't succeed without network, so needs a mock process executor
Might still try to use actual commands for error cases
Simulate common errors (e.g. net asking for pw)
Black box test of interface with input permutations that provide a large coverage
AuthenticateUser
user_principal_name (empty, contains ;rm -rf, several @s and dots, bad, good)
password_fd (invalid, empty, bad, good)
Parsing account id from 'objectGUID : 12345'
Test that krb5.conf is actually used
JoinMachine
machine_name (empty, ;rm -rf, is actually a user name a@b.c, bad, good)
user_principal_name (empty, contains ;rm -rf, several @s and dots, bad, good)
password_fd (invalid, empty, bad, good)
FetchUserGpos
account_id (empty, bad, good)
FetchDeviceGpos
GetMachineAndRealmName
UpdateDomainControllerName
WriteSmbConf
WriteKrb5Conf
GetGpoList
DownloadGpos

 

Comment 1 Deleted

Components: Enterprise
Labels: -M-56 M-57
Owner: ljusten@chromium.org
Status: Assigned
Summary: authpolicy: Add unit test for SambaInterface (was: authpolicy: Add unit test for UserPolicyEncoder)

Comment 3 Deleted

Description: Show this description

Comment 5 by tnagel@chromium.org, Nov 21 2016

Labels: V1
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 13 2016

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 13 2016

Labels: Enterprise-Triaged
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2fc19bc37c27665b781b8a6b634c720ce7f4fc45

commit 2fc19bc37c27665b781b8a6b634c720ce7f4fc45
Author: Lutz Justen <ljusten@chromium.org>
Date: Thu Feb 02 20:29:40 2017

authpolicy: Refactor samba file paths

Introduces a simple path service to SambaInterface that allows
overriding paths in a derived class for unit testing.

BUG= chromium:664441 
TEST=Compiles, tested all functionality, checked encryption types and file permissions

Change-Id: If8b5e75cb6c487437e0899b34d4db0242ac78529
Reviewed-on: https://chromium-review.googlesource.com/433761
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>

[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/authpolicy.gyp
[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/authpolicy.h
[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/samba_interface.cc
[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/samba_interface.h
[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/constants.h
[add] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/path_service.h
[add] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/path_service.cc
[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/authpolicy.cc
[modify] https://crrev.com/2fc19bc37c27665b781b8a6b634c720ce7f4fc45/authpolicy/authpolicy_main.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74

commit d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74
Author: Lutz Justen <ljusten@chromium.org>
Date: Mon Feb 13 00:53:01 2017

authpolicy: Refactor SambaInterface

Moves static methods into SambaInterface, reducing the amount of params
that need to be passed. Promotes some static variables to state variables.
Gets rid of the authpolicy::constants namespace, it's not necessary.

BUG= chromium:664441 
TEST=Tested all functionality on device, ran tests

Change-Id: I1efe6960150db8513e08f8633a0baa9209842ce0
Reviewed-on: https://chromium-review.googlesource.com/435143
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>

[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/process_executor.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/samba_interface_internal_unittest.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/policy/device_policy_encoder.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/policy/preg_policy_encoder.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/process_executor_unittest.cc
[add] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/platform_helper.h
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/policy/policy_encoder_helper.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/authpolicy_main.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/etc/init/authpolicyd.conf
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/policy/policy_encoder_helper.h
[rename] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/platform_helper.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/samba_interface.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/samba_interface_internal.h
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/authpolicy_parser_main.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/constants.h
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/authpolicy.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/policy/preg_policy_encoder.h
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/proto/authpolicy_containers.proto
[delete] https://crrev.com/eab2536af24dde121c1154b7907be753f14b5f67/authpolicy/pipe_helper.h
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/authpolicy.gyp
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/samba_interface_internal.cc
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/samba_interface.h
[modify] https://crrev.com/d7b00fd7d70c339ccbe4a5c0a34fe06ee1b79c74/authpolicy/authpolicy.h

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ea08ff241f886aadbde56020396ea3389ee67e46

commit ea08ff241f886aadbde56020396ea3389ee67e46
Author: Lutz Justen <ljusten@chromium.org>
Date: Tue Feb 14 22:50:38 2017

common-mk: Provide OUT env variable in platform2 tests

Passes through OUT and removes the sysroot (build/<board>), so that
the OUT folder can be easily accessed by unit tests. This is useful
when tests compile their own custom test binaries. These binaries
are written to the OUT folder or a subfolder thereof.

BUG= chromium:664441 
TEST=Verified that OUT is available in a test.

Change-Id: I5703a5df5db6e2e18f68c56a8c6fd0d3dcb6c50a
Reviewed-on: https://chromium-review.googlesource.com/439144
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/ea08ff241f886aadbde56020396ea3389ee67e46/common-mk/platform2_test.py

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/c2ee1cabf8a434a35024ca2c39c474ceeb6e3f4b

commit c2ee1cabf8a434a35024ca2c39c474ceeb6e3f4b
Author: Lutz Justen <ljusten@chromium.org>
Date: Tue Feb 21 16:49:38 2017

authpolicy: Disable authpolicy on x86

So far, only Samba was disabled on x86, but authpolicy was enabled.
Since authpolicy is unusable without Samba, though, it is cleaner
to disable it. Another reason to disable it is that unit tests are
failing because of a seccomp crash (filters have syscalls not present
on x86).

This change is implemented in chromeos-chrome-9999.ebuild to work
around an issue in the build system and will be moved to
package.use.mask when the ebuild is marked stable.

BUG= chromium:664441 
TEST=build_packages for enguarde

Change-Id: Ibc310050aa05a9457432b135f53445d57a215fe3
Reviewed-on: https://chromium-review.googlesource.com/443589
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>

[modify] https://crrev.com/c2ee1cabf8a434a35024ca2c39c474ceeb6e3f4b/profiles/arch/arm/package.use.mask
[modify] https://crrev.com/c2ee1cabf8a434a35024ca2c39c474ceeb6e3f4b/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2

commit 0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2
Author: Lutz Justen <ljusten@chromium.org>
Date: Tue Feb 21 16:49:39 2017

authpolicy: Add authpolicy integration test

Adds an integration test that calls authpolicy's D-Bus interface
methods and verifies the output. Uses stub implementations of net,
kinit and smbclient in order to simulate valid network response.

BUG= chromium:664441 
TEST=Compiles, tests work fine

Change-Id: I2bb3e106934e3a145684fc5756280311e6f68792
Reviewed-on: https://chromium-review.googlesource.com/435278
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>

[add] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/stub_common.h
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/authpolicy.gyp
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/platform_helper.cc
[add] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/authpolicy_unittest.cc
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/authpolicy.h
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/platform_helper.h
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/samba_interface.cc
[add] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/stub_common.cc
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/proto/authpolicy_containers.proto
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/authpolicy_parser_main.cc
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/samba_interface.h
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/constants.h
[add] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/stub_kinit_main.cc
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/path_service.h
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/authpolicy.cc
[add] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/stub_net_main.cc
[modify] https://crrev.com/0a1ccc61d0359ddbaae5b9178f3fa0041682f6c2/authpolicy/authpolicy_main.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a63e47c7fae289dedae6c23107dedb0cb58dea69

commit a63e47c7fae289dedae6c23107dedb0cb58dea69
Author: Lutz Justen <ljusten@chromium.org>
Date: Sat Mar 25 02:38:01 2017

authpolicy: Add more authpolicy integration tests

Adds tests for
- user authentication error behavior,
- domain join error behavior and
- failing device policy fetch if not joined.

BUG= chromium:664441 
TEST=Compiles, tests work fine

Change-Id: I9072ae0673017df428870c5b6f596854a66e4dc1
Reviewed-on: https://chromium-review.googlesource.com/444751
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/authpolicy.gyp
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/authpolicy_unittest.cc
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/authpolicy.h
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/stub_common.h
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/samba_interface.cc
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/stub_kinit_main.cc
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/samba_interface.h
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/stub_common.cc
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/authpolicy.cc
[modify] https://crrev.com/a63e47c7fae289dedae6c23107dedb0cb58dea69/authpolicy/stub_net_main.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/20ecdf737bb8e6267f9438ccb3574d02c01f4008

commit 20ecdf737bb8e6267f9438ccb3574d02c01f4008
Author: Lutz Justen <ljusten@chromium.org>
Date: Sat Apr 01 20:13:53 2017

authpolicy: Add even more authpolicy integration tests

Adds tests for
- policy fetch with actual policy data,
- policy override behavior and
- error behavior for policy fetch.

BUG= chromium:664441 
TEST=Compiles, tests work fine

Change-Id: I00f9d21694faa3ab5b185254893265cc91ca5af2
Reviewed-on: https://chromium-review.googlesource.com/445899
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Tested-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[add] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/stub_smbclient_main.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/authpolicy.gyp
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/policy/policy_encoder_helper.h
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/stub_net_main.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/authpolicy_unittest.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/authpolicy.h
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/stub_common.h
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/stub_common.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/samba_interface_internal.h
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/constants.h
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/stub_kinit_main.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/authpolicy.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/policy/policy_encoder_helper.cc
[modify] https://crrev.com/20ecdf737bb8e6267f9438ccb3574d02c01f4008/authpolicy/samba_interface_internal.cc

Labels: Pri-1
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e4ee9237ad0102f33210bef1dacd2d823fcfe59a

commit e4ee9237ad0102f33210bef1dacd2d823fcfe59a
Author: Lutz Justen <ljusten@chromium.org>
Date: Thu Apr 13 03:22:34 2017

authpolicy: Test authpolicy metrics

Adds checks to AuthPolicyTest that AuthPolicyMetrics is called the
expected number of times. In particular, this checks that KDC retry
is actually working as intended.

BUG= chromium:664441 
TEST=Compiles, tests work fine

Change-Id: I09de41102dc5c7ebac8ea4f119fdc67fca53cc78
Reviewed-on: https://chromium-review.googlesource.com/463269
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Tested-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[modify] https://crrev.com/e4ee9237ad0102f33210bef1dacd2d823fcfe59a/authpolicy/authpolicy_unittest.cc
[modify] https://crrev.com/e4ee9237ad0102f33210bef1dacd2d823fcfe59a/authpolicy/authpolicy.h
[modify] https://crrev.com/e4ee9237ad0102f33210bef1dacd2d823fcfe59a/authpolicy/authpolicy_metrics.h

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
bulk Verify of Chromad V1 bugs

Sign in to add a comment