New issue
Advanced search Search tips

Issue 727260 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

authpolicy: Fix broken policy fetch

Project Member Reported by ljusten@chromium.org, May 29 2017

Issue description

Policy fetch fails with this error:

INFO session_manager[22936]: [INFO:policy_service.cc(182)] Persisted policy to disk.
ERR authpolicyd[23014]: InvokeHelper(...): Domain=dbus, Code=org.freedesktop.DBus.Error.InvalidArgs, Message=Too few parameters in a method call
ERR authpolicyd[23014]: Call to StoreUnsignedPolicyForUser failed. Too few parameters in a method call

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 30 2017

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

commit 2809691aaabdf194ed87dff0d43c7b906a7e6a8d
Author: Lutz Justen <ljusten@chromium.org>
Date: Tue May 30 16:03:29 2017

authpolicy: Fix broken policy fetch

The bool 'true' in the response callback from Session Manager's
StorePolicy* methods was removed recently[1] and broke authpolicy.
This CL properly handles the new signature in authpolicy. Since we
don't have autotests yet, this wasn't caught in tests (planned for
M61).

[1] https://chromium-review.googlesource.com/c/512464/2/login_manager/session_manager_dbus_adaptor.cc#167

BUG= chromium:727260 
TEST=Tested both error and non-error case on device.

Change-Id: Ie8664a562f3ea03bc8c7bee62bfdb4f575ed803d
Reviewed-on: https://chromium-review.googlesource.com/517784
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/2809691aaabdf194ed87dff0d43c7b906a7e6a8d/authpolicy/authpolicy.cc
[modify] https://crrev.com/2809691aaabdf194ed87dff0d43c7b906a7e6a8d/authpolicy/authpolicy_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 30 2017

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

commit 2809691aaabdf194ed87dff0d43c7b906a7e6a8d
Author: Lutz Justen <ljusten@chromium.org>
Date: Tue May 30 16:03:29 2017

authpolicy: Fix broken policy fetch

The bool 'true' in the response callback from Session Manager's
StorePolicy* methods was removed recently[1] and broke authpolicy.
This CL properly handles the new signature in authpolicy. Since we
don't have autotests yet, this wasn't caught in tests (planned for
M61).

[1] https://chromium-review.googlesource.com/c/512464/2/login_manager/session_manager_dbus_adaptor.cc#167

BUG= chromium:727260 
TEST=Tested both error and non-error case on device.

Change-Id: Ie8664a562f3ea03bc8c7bee62bfdb4f575ed803d
Reviewed-on: https://chromium-review.googlesource.com/517784
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/2809691aaabdf194ed87dff0d43c7b906a7e6a8d/authpolicy/authpolicy.cc
[modify] https://crrev.com/2809691aaabdf194ed87dff0d43c7b906a7e6a8d/authpolicy/authpolicy_unittest.cc

Oops, sorry for breakage, and thank you for quick fix.
Really not your fault, we owe the integration test for that.
Labels: Merge-Request-60
Project Member

Comment 6 by sheriffbot@chromium.org, Jun 1 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 1 2017

Labels: merge-merged-release-R60-9592.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/870962ecac2d09f022a64d2263971b3b267e9008

commit 870962ecac2d09f022a64d2263971b3b267e9008
Author: Lutz Justen <ljusten@chromium.org>
Date: Thu Jun 01 17:16:22 2017

authpolicy: Fix broken policy fetch

The bool 'true' in the response callback from Session Manager's
StorePolicy* methods was removed recently[1] and broke authpolicy.
This CL properly handles the new signature in authpolicy. Since we
don't have autotests yet, this wasn't caught in tests (planned for
M61).

[1] https://chromium-review.googlesource.com/c/512464/2/login_manager/session_manager_dbus_adaptor.cc#167

BUG= chromium:727260 
TEST=Tested both error and non-error case on device.

Change-Id: Ie8664a562f3ea03bc8c7bee62bfdb4f575ed803d
Reviewed-on: https://chromium-review.googlesource.com/517784
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
(cherry picked from commit 2809691aaabdf194ed87dff0d43c7b906a7e6a8d)
Reviewed-on: https://chromium-review.googlesource.com/521483
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Trybot-Ready: Lutz Justen <ljusten@chromium.org>

[modify] https://crrev.com/870962ecac2d09f022a64d2263971b3b267e9008/authpolicy/authpolicy.cc
[modify] https://crrev.com/870962ecac2d09f022a64d2263971b3b267e9008/authpolicy/authpolicy_unittest.cc

Project Member

Comment 8 by sheriffbot@chromium.org, Jun 5 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Approved -Merge-Approved-60
Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
bulk Verify of older or not-user-facing Chromad bugs

Sign in to add a comment