Storage Framework

Merge lp:~jamesh/storage-framework/provider-auth-error into lp:storage-framework/devel

  1. provider-auth-error
  2. Merge into devel
Proposed by James Henstridge
Status: Merged
Approved by: Michi Henning
Approved revision: 114
Merged at revision: 105
Proposed branch: lp:~jamesh/storage-framework/provider-auth-error
Merge into: lp:storage-framework/devel
Diff against target: 658 lines (+292/-48)
17 files modified
debian/changelog (+3/-0)
include/unity/storage/provider/Exceptions.h (+30/-0)
include/unity/storage/provider/internal/AccountData.h (+3/-3)
include/unity/storage/provider/internal/Handler.h (+5/-1)
include/unity/storage/qt/StorageError.h (+3/-2)
src/provider/Exceptions.cpp (+7/-0)
src/provider/internal/AccountData.cpp (+34/-9)
src/provider/internal/Handler.cpp (+54/-2)
src/provider/internal/ProviderInterface.cpp (+1/-12)
src/provider/internal/ServerImpl.cpp (+1/-0)
src/provider/internal/TestServerImpl.cpp (+1/-0)
src/qt/internal/StorageErrorImpl.cpp (+2/-1)
src/qt/internal/unmarshal_error.cpp (+1/-0)
tests/provider-ProviderInterface/ProviderInterface_test.cpp (+103/-0)
tests/utils/ProviderFixture.cpp (+3/-2)
tests/utils/ProviderFixture.h (+2/-1)
tests/utils/fake-online-accounts-daemon.py (+39/-15)
To merge this branch: bzr merge lp:~jamesh/storage-framework/provider-auth-error
Reviewer Review Type Date Requested Status
Michi Henning (community) Approve
unity-api-1-bot continuous-integration Approve
Review via email: mp+315429@code.launchpad.net

Commit message

Add an UnauthorizedException that providers can use to signal when the credentials provided by online-accounts are invalid or have expired.

Description of the change

Add an UnauthorizedException that providers can use to signal when the credentials provided by online-accounts are invalid or have expired.

If a provider throws this error, the request will be retried a single time after attempting to authenticate a second time while asking for stored credentials to be invalidated.

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:109
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/239/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1503/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1510
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1288
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1288/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1288
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1288/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1288
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1288/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1288/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1288
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1288/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1288
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1288/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/239/rebuild

review: Needs Fixing (continuous-integration)
lp:~jamesh/storage-framework/provider-auth-error updated
110. By James Henstridge

Don't call the ProviderBase implementation if authentication failed.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:110
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/240/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1509/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1516
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1294
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1294/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1294
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1294/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1294
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1294/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1294
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1294/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1294/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1294
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1294/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/240/rebuild

review: Needs Fixing (continuous-integration)
lp:~jamesh/storage-framework/provider-auth-error updated
111. By James Henstridge

Fix up handling of invalid credentials so that they actually get
re-requested on failure.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:111
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/241/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1511
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1518
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1296
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1296/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1296
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1296/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1296
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1296/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1296
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1296/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1296
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1296/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1296
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1296/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/241/rebuild

review: Approve (continuous-integration)
lp:~jamesh/storage-framework/provider-auth-error updated
112. By James Henstridge

Merge from devel

113. By James Henstridge

Update debian/changelog

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:113
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/242/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1518/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1525
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1303/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1303
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1303/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1303
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1303/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1303
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1303/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1303/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1303
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1303/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/242/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:113
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/243/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1522/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1529
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1307/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1307
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1307/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1307
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1307/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1307
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1307/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1307/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1307
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1307/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/243/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:113
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/244/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1523
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1530
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1308
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1308/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1308
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1308/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1308
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1308/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1308
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1308/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1308
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1308/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1308
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1308/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/244/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

This looks good! I left a comment on the doc inline.

lp:~jamesh/storage-framework/provider-auth-error updated
114. By James Henstridge

Update documentation based on Michi's review.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:114
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/245/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1527
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1534
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1312
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1312/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1312
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1312/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1312
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1312/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1312
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1312/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1312
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1312/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1312
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1312/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/245/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2016-12-20 03:48:43 +0000
3+++ debian/changelog 2017-01-27 09:28:36 +0000
4@@ -1,6 +1,9 @@
5 storage-framework (0.3-0ubuntu1) UNRELEASED; urgency=medium
6
7 * Make providers exit after 30 seconds of inactivity. (LP: #1616758)
8+ * Add UnauthorizedException to the provider library, so the provider
9+ can trigger reauthentication of the account and have the request
10+ restarted. (LP: #1616756)
11
12 -- James Henstridge <james.henstridge@canonical.com> Tue, 20 Dec 2016 11:46:21 +0800
13
14
15=== modified file 'include/unity/storage/provider/Exceptions.h'
16--- include/unity/storage/provider/Exceptions.h 2016-08-05 05:37:23 +0000
17+++ include/unity/storage/provider/Exceptions.h 2017-01-27 09:28:36 +0000
18@@ -105,7 +105,37 @@
19 };
20
21 /**
22+\brief Indicates that an operation failed because the authentication credentials are invalid or expired.
23+
24+A provider implementation must throw this exception if it cannot reach
25+its provider because the credentials are invalid. Do not throw this
26+exception if the credentials are valid, but an operation failed due to
27+insufficient permission for an item (such as an attempt to write to a
28+read-only file).
29+
30+Typically, this will cause the request to be retried after refreshing
31+the authentication credentials, but may be returned to the client on
32+repeated failures.
33+
34+\see PermissionException
35+*/
36+class UNITY_STORAGE_EXPORT UnauthorizedException : public StorageException
37+{
38+public:
39+ UnauthorizedException(std::string const& error_message);
40+ ~UnauthorizedException();
41+};
42+
43+/**
44 \brief Indicates that an operation failed because of a permission problem.
45+
46+A provider implementation must throw this exception if it can
47+authenticate with its provider, but the provider denied the operation
48+due to insufficient permission for an item (such as an attempt to
49+write to a read-only file). Do not throw this exception for failure to
50+authenticate with the provider.
51+
52+\see UnauthorizedException
53 */
54 class UNITY_STORAGE_EXPORT PermissionException : public StorageException
55 {
56
57=== modified file 'include/unity/storage/provider/internal/AccountData.h'
58--- include/unity/storage/provider/internal/AccountData.h 2016-12-20 02:31:23 +0000
59+++ include/unity/storage/provider/internal/AccountData.h 2017-01-27 09:28:36 +0000
60@@ -66,7 +66,7 @@
61 QObject* parent=nullptr);
62 virtual ~AccountData();
63
64- void authenticate(bool interactive);
65+ void authenticate(bool interactive, bool invalidate_cache=false);
66 bool has_credentials();
67 Credentials const& credentials();
68
69@@ -90,9 +90,9 @@
70 QPointer<OnlineAccounts::Account> const account_;
71 std::unique_ptr<OnlineAccounts::PendingCallWatcher> auth_watcher_;
72 bool authenticating_interactively_ = false;
73+ bool authenticating_invalidate_cache_ = false;
74
75- bool credentials_valid_ = false;
76- Credentials credentials_;
77+ Credentials credentials_ = boost::blank();
78
79 Q_DISABLE_COPY(AccountData)
80 };
81
82=== modified file 'include/unity/storage/provider/internal/Handler.h'
83--- include/unity/storage/provider/internal/Handler.h 2016-12-20 02:49:22 +0000
84+++ include/unity/storage/provider/internal/Handler.h 2017-01-27 09:28:36 +0000
85@@ -53,11 +53,12 @@
86 Callback const& callback,
87 QDBusConnection const& bus, QDBusMessage const& message);
88
89-public Q_SLOTS:
90 void begin();
91
92 private Q_SLOTS:
93+ void on_authenticated();
94 void credentials_received();
95+ void handle_unauthorized(std::exception_ptr ep);
96 void send_reply();
97
98 Q_SIGNALS:
99@@ -76,6 +77,7 @@
100 boost::future<void> reply_future_;
101 Context context_;
102 QDBusMessage reply_;
103+ bool retry_ = false;
104
105 Q_DISABLE_COPY(Handler)
106 };
107@@ -84,3 +86,5 @@
108 }
109 }
110 }
111+
112+Q_DECLARE_METATYPE(std::exception_ptr)
113
114=== modified file 'include/unity/storage/qt/StorageError.h'
115--- include/unity/storage/qt/StorageError.h 2016-10-12 06:05:29 +0000
116+++ include/unity/storage/qt/StorageError.h 2017-01-27 09:28:36 +0000
117@@ -58,8 +58,9 @@
118
119 enum Type
120 {
121- NoError, LocalCommsError, RemoteCommsError, RuntimeDestroyed, NotExists, Exists, Conflict,
122- PermissionDenied, Cancelled, LogicError, InvalidArgument, ResourceError, QuotaExceeded,
123+ NoError, LocalCommsError, RemoteCommsError, RuntimeDestroyed,
124+ NotExists, Exists, Conflict, PermissionDenied, Cancelled, LogicError,
125+ InvalidArgument, ResourceError, QuotaExceeded, Unauthorized,
126 __LAST_STORAGE_ERROR
127 };
128 Q_ENUMS(Type)
129
130=== modified file 'src/provider/Exceptions.cpp'
131--- src/provider/Exceptions.cpp 2016-08-05 05:37:23 +0000
132+++ src/provider/Exceptions.cpp 2017-01-27 09:28:36 +0000
133@@ -97,6 +97,13 @@
134
135 ConflictException::~ConflictException() = default;
136
137+UnauthorizedException::UnauthorizedException(string const& error_message)
138+ : StorageException("UnauthorizedException", error_message)
139+{
140+}
141+
142+UnauthorizedException::~UnauthorizedException() = default;
143+
144 PermissionException::PermissionException(string const& error_message)
145 : StorageException("PermissionException", error_message)
146 {
147
148=== modified file 'src/provider/internal/AccountData.cpp'
149--- src/provider/internal/AccountData.cpp 2016-12-20 02:31:23 +0000
150+++ src/provider/internal/AccountData.cpp 2017-01-27 09:28:36 +0000
151@@ -68,23 +68,48 @@
152 return *jobs_;
153 }
154
155-void AccountData::authenticate(bool interactive)
156+void AccountData::authenticate(bool interactive, bool invalidate_cache)
157 {
158- // If there is an existing authenticating session running, use
159- // that existing session (unless it is a non-interactive session
160- // and we've requested interactivity).
161- if (auth_watcher_ && (authenticating_interactively_ || !interactive))
162+ // If there is an existing authentication session running, check
163+ // if it matches our requirements.
164+ if (auth_watcher_)
165 {
166- return;
167+ if (invalidate_cache)
168+ {
169+ // If invalidate_cache has been requested, the existing
170+ // session must also be invalidating the cache.
171+ if (authenticating_invalidate_cache_)
172+ {
173+ return;
174+ }
175+ }
176+ else if (interactive)
177+ {
178+ // If interactive has been requested, the existing session
179+ // must also be interactive.
180+ if (authenticating_interactively_)
181+ {
182+ return;
183+ }
184+ }
185+ else
186+ {
187+ // Otherwise, any session will do.
188+ return;
189+ }
190 }
191
192 authenticating_interactively_ = interactive;
193- credentials_valid_ = false;
194+ authenticating_invalidate_cache_ = invalidate_cache;
195 credentials_ = boost::blank();
196
197 OnlineAccounts::AuthenticationData auth_data(
198 account_->authenticationMethod());
199 auth_data.setInteractive(interactive);
200+ if (invalidate_cache)
201+ {
202+ auth_data.invalidateCachedReply();
203+ }
204 OnlineAccounts::PendingCall call = account_->authenticate(auth_data);
205 auth_watcher_.reset(new OnlineAccounts::PendingCallWatcher(call));
206 connect(auth_watcher_.get(), &OnlineAccounts::PendingCallWatcher::finished,
207@@ -93,7 +118,8 @@
208
209 bool AccountData::has_credentials()
210 {
211- return credentials_valid_;
212+ // variant index 0 is boost::blank
213+ return credentials_.which() != 0;
214 }
215
216 Credentials const& AccountData::credentials()
217@@ -104,7 +130,6 @@
218 void AccountData::on_authenticated()
219 {
220 credentials_ = boost::blank();
221- credentials_valid_ = true;
222 switch (account_->authenticationMethod()) {
223 case OnlineAccounts::AuthenticationMethodOAuth1:
224 {
225
226=== modified file 'src/provider/internal/Handler.cpp'
227--- src/provider/internal/Handler.cpp 2016-12-20 02:49:22 +0000
228+++ src/provider/internal/Handler.cpp 2017-01-27 09:28:36 +0000
229@@ -56,6 +56,35 @@
230
231 void Handler::begin()
232 {
233+ // If we have already retrieved credentials from OnlineAccounts,
234+ // and we aren't retrying the request, go to on_authenticated
235+ // immediately.
236+ if (account_->has_credentials() && !retry_)
237+ {
238+ on_authenticated();
239+ return;
240+ }
241+
242+ // Otherwise, try to authenticate and wait for the result.
243+ account_->authenticate(true, retry_);
244+ connect(account_.get(), &AccountData::authenticated,
245+ this, &Handler::on_authenticated);
246+}
247+
248+void Handler::on_authenticated()
249+{
250+ disconnect(account_.get(), &AccountData::authenticated,
251+ this, &Handler::on_authenticated);
252+ if (!account_->has_credentials())
253+ {
254+ string msg = "Handler::begin(): could not retrieve account credentials";
255+ qDebug() << QString::fromStdString(msg);
256+ auto ep = make_exception_ptr(UnauthorizedException(msg));
257+ marshal_exception(ep);
258+ QMetaObject::invokeMethod(this, "send_reply", Qt::QueuedConnection);
259+ return;
260+ }
261+
262 // Need to put security check in here.
263 auto peer_future = account_->dbus_peer().get(message_.service());
264 creds_future_ = peer_future.then(
265@@ -72,9 +101,9 @@
266 }
267 else
268 {
269- string msg = "Handler::begin(): could not retrieve credentials";
270+ string msg = "Handler::begin(): could not retrieve D-Bus peer credentials";
271 qDebug() << QString::fromStdString(msg);
272- auto ep = make_exception_ptr(PermissionException(msg));
273+ auto ep = make_exception_ptr(UnauthorizedException(msg));
274 marshal_exception(ep);
275 QMetaObject::invokeMethod(this, "send_reply",
276 Qt::QueuedConnection);
277@@ -104,6 +133,13 @@
278 {
279 reply_ = f.get();
280 }
281+ catch (UnauthorizedException const& e)
282+ {
283+ QMetaObject::invokeMethod(this, "handle_unauthorized",
284+ Qt::QueuedConnection,
285+ Q_ARG(std::exception_ptr, current_exception()));
286+ return;
287+ }
288 catch (std::exception const& e)
289 {
290 marshal_exception(current_exception());
291@@ -112,6 +148,22 @@
292 });
293 }
294
295+void Handler::handle_unauthorized(exception_ptr ep)
296+{
297+ if (retry_)
298+ {
299+ // We've already retried once, so send error out as is.
300+ marshal_exception(ep);
301+ send_reply();
302+ }
303+ else
304+ {
305+ // Otherwise, restart the request with the retry_ flag set.
306+ retry_ = true;
307+ begin();
308+ }
309+}
310+
311 void Handler::send_reply()
312 {
313 bus_.send(reply_);
314
315=== modified file 'src/provider/internal/ProviderInterface.cpp'
316--- src/provider/internal/ProviderInterface.cpp 2016-12-20 03:00:07 +0000
317+++ src/provider/internal/ProviderInterface.cpp 2017-01-27 09:28:36 +0000
318@@ -66,18 +66,7 @@
319 new Handler(account_, callback, connection(), message()));
320 connect(handler.get(), &Handler::finished, this, &ProviderInterface::request_finished);
321 setDelayedReply(true);
322- // If we haven't retrieved the authentication details from
323- // OnlineAccounts, delay processing the handler until then.
324- if (account_->has_credentials())
325- {
326- handler->begin();
327- }
328- else
329- {
330- account_->authenticate(true);
331- connect(account_.get(), &AccountData::authenticated,
332- handler.get(), &Handler::begin);
333- }
334+ handler->begin();
335 requests_.emplace(handler.get(), std::move(handler));
336 }
337
338
339=== modified file 'src/provider/internal/ServerImpl.cpp'
340--- src/provider/internal/ServerImpl.cpp 2016-12-20 07:29:02 +0000
341+++ src/provider/internal/ServerImpl.cpp 2017-01-27 09:28:36 +0000
342@@ -48,6 +48,7 @@
343 , service_id_(account_service_id)
344 , trace_message_handler_("storage_provider")
345 {
346+ qRegisterMetaType<std::exception_ptr>();
347 qDBusRegisterMetaType<Item>();
348 qDBusRegisterMetaType<std::vector<Item>>();
349 }
350
351=== modified file 'src/provider/internal/TestServerImpl.cpp'
352--- src/provider/internal/TestServerImpl.cpp 2016-12-20 02:31:23 +0000
353+++ src/provider/internal/TestServerImpl.cpp 2017-01-27 09:28:36 +0000
354@@ -54,6 +54,7 @@
355 : connection_(connection), object_path_(object_path),
356 inactivity_timer_(make_shared<InactivityTimer>(TIMEOUT))
357 {
358+ qRegisterMetaType<std::exception_ptr>();
359 qDBusRegisterMetaType<Item>();
360 qDBusRegisterMetaType<std::vector<Item>>();
361
362
363=== modified file 'src/qt/internal/StorageErrorImpl.cpp'
364--- src/qt/internal/StorageErrorImpl.cpp 2016-10-13 06:48:11 +0000
365+++ src/qt/internal/StorageErrorImpl.cpp 2017-01-27 09:28:36 +0000
366@@ -48,7 +48,8 @@
367 QStringLiteral("Cancelled"),
368 QStringLiteral("LogicError"),
369 QStringLiteral("InvalidArgument"),
370- QStringLiteral("ResourceError")
371+ QStringLiteral("ResourceError"),
372+ QStringLiteral("Unauthorized"),
373 };
374
375 } // namespace
376
377=== modified file 'src/qt/internal/unmarshal_error.cpp'
378--- src/qt/internal/unmarshal_error.cpp 2016-09-29 03:17:56 +0000
379+++ src/qt/internal/unmarshal_error.cpp 2017-01-27 09:28:36 +0000
380@@ -82,6 +82,7 @@
381 { "NotExistsException", make_error<StorageError::Type::NotExists> },
382 { "ExistsException", make_error<StorageError::Type::Exists> },
383 { "ConflictException", make_error<StorageError::Type::Conflict> },
384+ { "UnauthorizedException", make_error<StorageError::Type::Unauthorized> },
385 { "PermissionException", make_error<StorageError::Type::PermissionDenied> },
386 { "CancelledException", make_error<StorageError::Type::Cancelled> },
387 { "LogicException", make_error<StorageError::Type::LogicError> },
388
389=== modified file 'tests/provider-ProviderInterface/ProviderInterface_test.cpp'
390--- tests/provider-ProviderInterface/ProviderInterface_test.cpp 2017-01-12 06:02:30 +0000
391+++ tests/provider-ProviderInterface/ProviderInterface_test.cpp 2017-01-27 09:28:36 +0000
392@@ -17,12 +17,14 @@
393 */
394
395 #include <unity/storage/internal/dbus_error.h>
396+#include <unity/storage/provider/Exceptions.h>
397 #include <unity/storage/provider/ProviderBase.h>
398 #include <unity/storage/provider/testing/TestServer.h>
399
400 #include "TestProvider.h"
401
402 #include <utils/ProviderFixture.h>
403+#include <utils/gtest_printer.h>
404
405 #include <gtest/gtest.h>
406 #include <OnlineAccounts/Account>
407@@ -45,6 +47,9 @@
408 using namespace std;
409 using unity::storage::ItemType;
410 using unity::storage::provider::ProviderBase;
411+using unity::storage::provider::Context;
412+using unity::storage::provider::PasswordCredentials;
413+using unity::storage::provider::UnauthorizedException;
414 using unity::storage::provider::testing::TestServer;
415
416 namespace {
417@@ -781,6 +786,104 @@
418 EXPECT_EQ(ItemType::file, item.type);
419 }
420
421+class ReauthenticateProvider : public TestProvider
422+{
423+ boost::future<ItemList> roots(vector<string> const& metadata_keys,
424+ Context const& ctx) override
425+ {
426+ Q_UNUSED(metadata_keys);
427+ auto password = boost::get<PasswordCredentials>(ctx.credentials).password;
428+ boost::promise<ItemList> p;
429+ p.set_value({
430+ {"root_id", {}, password, "etag", ItemType::root, {}}
431+ });
432+ return p.get_future();
433+ }
434+
435+ boost::future<Item> metadata(string const& item_id,
436+ vector<string> const& metadata_keys,
437+ Context const& ctx) override
438+ {
439+ Q_UNUSED(metadata_keys);
440+ auto password = boost::get<PasswordCredentials>(ctx.credentials).password;
441+ boost::promise<Item> p;
442+ if (password != "refresh")
443+ {
444+ p.set_exception(UnauthorizedException("bad password"));
445+ }
446+ else
447+ {
448+ p.set_value(
449+ {item_id, {"root_id"}, password, "etag", ItemType::file, {}});
450+ }
451+ return p.get_future();
452+ }
453+
454+ boost::future<ItemList> lookup(string const& parent_id, string const& name,
455+ vector<string> const& metadata_keys,
456+ Context const& ctx) override
457+ {
458+ Q_UNUSED(parent_id);
459+ Q_UNUSED(name);
460+ Q_UNUSED(metadata_keys);
461+ Q_UNUSED(ctx);
462+ boost::promise<ItemList> p;
463+ p.set_exception(UnauthorizedException("bad password"));
464+ return p.get_future();
465+ }
466+};
467+
468+TEST_F(ProviderInterfaceTest, need_interactive_auth)
469+{
470+ // Account #10 fails to authenticate unless done interactively
471+ set_provider(unique_ptr<ProviderBase>(new ReauthenticateProvider), 10);
472+
473+ auto reply = client_->Roots(QList<QString>());
474+ wait_for(reply);
475+ ASSERT_TRUE(reply.isValid()) << reply.error().message().toStdString();
476+ EXPECT_EQ(1, reply.value().size());
477+ auto root = reply.value()[0];
478+ // Password returned as item name
479+ EXPECT_EQ("interactive", root.name);
480+}
481+
482+TEST_F(ProviderInterfaceTest, unauthorized_exception_causes_refresh)
483+{
484+ set_provider(unique_ptr<ProviderBase>(new ReauthenticateProvider), 10);
485+
486+ // The Metadata() call will throw UnauthorizedException unless the
487+ // password is "refresh". This will cause the request to be
488+ // retried after authenticating a second time while invalidating
489+ // stored credentials.
490+ auto reply = client_->Metadata("item_id", QList<QString>());
491+ wait_for(reply);
492+ ASSERT_TRUE(reply.isValid()) << reply.error().message().toStdString();
493+ auto item = reply.value();
494+ // Password returned as item name
495+ EXPECT_EQ("refresh", item.name);
496+}
497+
498+TEST_F(ProviderInterfaceTest, always_unauthorized)
499+{
500+ set_provider(unique_ptr<ProviderBase>(new ReauthenticateProvider), 10);
501+ // lookup() will always throw UnauthorizedException. Rather than
502+ // looping endlessly, the exception is returned to the client.
503+ auto reply = client_->Lookup("parent_id", "name", QList<QString>());
504+ wait_for(reply);
505+ ASSERT_FALSE(reply.isValid());
506+ EXPECT_EQ(PROVIDER_ERROR + "UnauthorizedException", reply.error().name());
507+}
508+
509+TEST_F(ProviderInterfaceTest, user_canceled_auth)
510+{
511+ // Account #11 always returns a UserCanceled error when trying to
512+ // authenticate.
513+ set_provider(unique_ptr<ProviderBase>(new ReauthenticateProvider), 11);
514+ auto reply = client_->Roots(QList<QString>());
515+ wait_for(reply);
516+ ASSERT_FALSE(reply.isValid());
517+ EXPECT_EQ(PROVIDER_ERROR + "UnauthorizedException", reply.error().name());
518+}
519
520 int main(int argc, char **argv)
521 {
522
523=== modified file 'tests/utils/ProviderFixture.cpp'
524--- tests/utils/ProviderFixture.cpp 2016-12-20 07:23:54 +0000
525+++ tests/utils/ProviderFixture.cpp 2017-01-27 09:28:36 +0000
526@@ -57,10 +57,11 @@
527 return dbus_->connection();
528 }
529
530-void ProviderFixture::set_provider(unique_ptr<ProviderBase>&& provider)
531+void ProviderFixture::set_provider(unique_ptr<ProviderBase>&& provider,
532+ unsigned int account_id)
533 {
534 account_manager_->waitForReady();
535- OnlineAccounts::Account* account = account_manager_->account(2, "oauth2-service");
536+ OnlineAccounts::Account* account = account_manager_->account(account_id);
537 ASSERT_NE(nullptr, account);
538
539 test_server_.reset(
540
541=== modified file 'tests/utils/ProviderFixture.h'
542--- tests/utils/ProviderFixture.h 2016-11-23 03:03:58 +0000
543+++ tests/utils/ProviderFixture.h 2017-01-27 09:28:36 +0000
544@@ -41,7 +41,8 @@
545 virtual void TearDown() override;
546
547 QDBusConnection const& connection() const;
548- void set_provider(std::unique_ptr<unity::storage::provider::ProviderBase>&& provider);
549+ void set_provider(std::unique_ptr<unity::storage::provider::ProviderBase>&& provider,
550+ unsigned int account_id = 2);
551 void wait_for(QDBusPendingCall const& call);
552 QString bus_name() const;
553 QString object_path() const;
554
555=== modified file 'tests/utils/fake-online-accounts-daemon.py'
556--- tests/utils/fake-online-accounts-daemon.py 2017-01-13 05:53:50 +0000
557+++ tests/utils/fake-online-accounts-daemon.py 2017-01-27 09:28:36 +0000
558@@ -41,7 +41,7 @@
559 self.token_secret = token_secret
560 self.signature_method = signature_method
561
562- def serialise(self):
563+ def serialise(self, interactive, invalidate):
564 return dbus.Dictionary({
565 "ConsumerKey": dbus.String(self.consumer_key),
566 "ConsumerSecret": dbus.String(self.consumer_secret),
567@@ -57,7 +57,7 @@
568 self.expires_in = expires_in
569 self.granted_scopes = granted_scopes
570
571- def serialise(self):
572+ def serialise(self, interactive, invalidate):
573 return dbus.Dictionary({
574 "AccessToken": dbus.String(self.access_token),
575 "ExpiresIn": dbus.Int32(self.expires_in),
576@@ -70,20 +70,36 @@
577 self.username = username
578 self.password = password
579
580- def serialise(self):
581+ def serialise(self, interactive, invalidate):
582 return dbus.Dictionary({
583 "Username": dbus.String(self.username),
584 "Password": dbus.String(self.password),
585 }, signature="sv")
586
587-# A version of Password that incorrectly serialises the credentials
588-# https://bugs.launchpad.net/bugs/1628473
589-class Password_Bug1628473(Password):
590- def serialise(self):
591- return dbus.Dictionary({
592- "UserName": dbus.String(self.username),
593- "Secret": dbus.String(self.password),
594- }, signature="sv")
595+class CredentialsError:
596+ def __init__(self, method, error):
597+ assert error in {"NoAccount", "UserCanceled",
598+ "PermissionDenied", "InteractionRequired"}
599+ self.method = method
600+ self.error = "com.ubuntu.OnlineAccounts.Error." + error
601+
602+ def serialise(self, interactive, invalidate):
603+ raise dbus.DBusException("Error", name=self.error)
604+
605+class CredentialsByMode:
606+ def __init__(self, noninteractive, interactive, refresh):
607+ self.method = noninteractive.method
608+ self.noninteractive = noninteractive
609+ self.interactive = interactive
610+ self.refresh = refresh
611+
612+ def serialise(self, interactive, invalidate):
613+ if invalidate:
614+ return self.refresh.serialise(interactive, invalidate)
615+ elif interactive:
616+ return self.interactive.serialise(interactive, invalidate)
617+ else:
618+ return self.noninteractive.serialise(interactive, invalidate)
619
620 class Account:
621 def __init__(self, account_id, name, service_id, credentials, settings=None):
622@@ -124,7 +140,7 @@
623 sys.stdout.flush()
624 for account in self.accounts:
625 if account.account_id == account_id and account.service_id == service_id:
626- return account.credentials.serialise()
627+ return account.credentials.serialise(interactive, invalidate)
628 else:
629 raise KeyError(repr((account_id, service_id)))
630
631@@ -136,7 +152,7 @@
632 for account in self.accounts:
633 if account.service_id == service_id:
634 return (account.serialise(),
635- account.credentials.serialise())
636+ account.credentials.serialise(True, False))
637 else:
638 raise KeyError(service_id)
639
640@@ -172,8 +188,16 @@
641 Account(3, "Password account", "password-service",
642 Password("user", "pass")),
643 Account(4, "Password host account", "password-host-service",
644- Password_Bug1628473("joe", "secret"),
645- {"host": "http://www.example.com/"}),
646+ Password("joe", "secret"),
647+ {"host": "http://www.example.com/"}),
648+ Account(10, "Mode dependent account", "mode-service",
649+ CredentialsByMode(
650+ noninteractive=CredentialsError(AUTH_PASSWORD, "InteractionRequired"),
651+ interactive=Password("user", "interactive"),
652+ refresh=Password("user", "refresh")),
653+ {"host": "http://www.example.com/"}),
654+ Account(11, "User cancel account", "user-cancel-service",
655+ CredentialsError(AUTH_PASSWORD, "UserCanceled")),
656 Account(42, "Fake test account", "storage-provider-test",
657 OAuth2("fake-test-access-token", 0, [])),
658 Account(99, "Fake mcloud account", "storage-provider-mcloud",

Subscribers

People subscribed via source and target branches