OAuth2 client initial contribution #5142

garytse · 2018-02-23T21:03:37Z

Support getting access token by username password, by authorization code.
Support refresh token
Support caching
No 3rd party library needed
Accounts migration support to be added EDIT: added now

kaikreuzer · 2018-02-25T16:47:04Z

Thanks for this contribution. Just a first comment: We already have an org.eclipse.smarthome.core.auth package in the core bundle, so I would say that this client does not need to be put into a dedicated bundle, but could rather be placed in a sub-package like org.eclipse.smarthome.core.auth.client (no need for "api" in the package name either).

kaikreuzer · 2018-02-26T09:49:40Z

but could rather be placed in a sub-package like org.eclipse.smarthome.core.auth.client

Ok, I just noticed that this only works for the api part, but not for the internal implementation as this would introduce a dependency from smarthome.core to Jetty bundles, which we probably do not want.

kaikreuzer · 2018-02-26T10:42:48Z

FTR: I have moved the implementation to a new bundle org.eclipse.smarthome.auth.oauth2client and the API into the smarthome.core bundle in the package org.eclipse.smarthome.core.auth.client.oauth2.

maggu2810

Haven't found any time to look at the code itself.
Just a short question: Shouldn't new contributions use annotations instead of OSGI-INF/*.xml files?

garytse · 2018-03-05T21:25:54Z

@maggu2810 At code level I am using annotations. Happy to follow if anyone can confirm to remove OSGI-INF/*.xml please?

kaikreuzer · 2018-03-05T21:53:20Z

@garytse Certainly so, see A.10 of our coding guidelines.

amitjoy · 2018-03-06T12:29:34Z

...t/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthClientServiceImpl.java

+ * @author Gary Tse - Initial contribution
+ *
+ */
+@Component(name = "org.eclipse.smarthome.auth.oauth2.client.OAuthClientService", servicefactory = true, property = {


configurationPid="org.eclipse.smarthome.any.pid", configurationPolicy=REQUIRE would enable the creation of multiple instances of OAuthClientServiceImpl.

If we do this, and if the component has a configuration policy of required as shown above, then we will get multiple instances of the component, with each instance potentially receiving different configuration data. DS will continue to maintain all the service references in our component of course.

amitjoy · 2018-03-06T12:31:08Z

...t/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthClientServiceImpl.java

+ *
+ */
+@Component(name = "org.eclipse.smarthome.auth.oauth2.client.OAuthClientService", servicefactory = true, property = {
+        "tokenExpiresInBuffer=60" })


You can also extract this as an externalized configuration through config admin and you can introduce a metatype XML descriptor for that as well.

amitjoy · 2018-03-06T12:33:04Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+     *
+     * It is only valid for one-time-only to get the access token.
+     *
+     * https://tools.ietf.org/html/rfc6749#section-4


@see https://tools.ietf.org/html/rfc6749#section-4

amitjoy · 2018-03-06T12:34:04Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+     * Redirect URI. This is read by the authorization server. The URI should be
+     * somewhere in the client application ready to receive the tokens.
+     *
+     * {@see https://tools.ietf.org/html/rfc6749#section-3.1.2}


without brackets

amitjoy · 2018-03-06T12:34:13Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+     * 
+     * If left null, a random state will be generated.
+     * 
+     * {@see https://tools.ietf.org/html/rfc6749#section-5.2}


without brackets

amitjoy · 2018-03-06T12:34:59Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+     * {@link https://tools.ietf.org/html/rfc6749#section-1.4}
+     *
+     * This token must be confidential in transit and storage.
+     * https://tools.ietf.org/html/rfc6749#section-10.3


@see https://tools.ietf.org/html/rfc6749#section-4

amitjoy · 2018-03-06T12:35:15Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+
+    /**
+     * Token type. e.g. Bearer, MAC
+     * {@see https://tools.ietf.org/html/rfc6749#section-7.1}


without curly braces

amitjoy · 2018-03-06T12:36:23Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+
+    /**
+     * Number of seconds that this OAuthToken is valid for since the time it was created.
+     * {@link https://tools.ietf.org/html/rfc6749#section-4.2.2}


@link is for inline methods, constants and so on but @see is for external links

amitjoy · 2018-03-06T12:36:41Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+     * the client by the resource owner. Unlike access tokens, refresh tokens are
+     * intended for use only with authorization servers and are never sent
+     * to resource servers.
+     * {@link https://tools.ietf.org/html/rfc6749#section-1.5}


@see without braces

amitjoy · 2018-03-06T12:36:48Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+     * {@link https://tools.ietf.org/html/rfc6749#section-1.5}
+     *
+     * This token must be confidential in transit and storage.
+     * {@link https://tools.ietf.org/html/rfc6749#section-10.4}


@see without braces

amitjoy · 2018-03-06T12:36:54Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+     * by the authorization server to inform the client of the scope of the access token
+     * issued.
+     *
+     * {@link https://tools.ietf.org/html/rfc6749#section-3.3}


@see without braces

amitjoy · 2018-03-06T12:37:01Z

...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthToken.java

+     * If the "state" parameter was present in the client authorization request.
+     * The exact value received from the client.
+     *
+     * {@link https://tools.ietf.org/html/rfc6749#section-4.2.2}


@see without braces

amitjoy · 2018-03-06T12:39:14Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthClientService.java

+     * then this call can be used to get a new acess token by using the refresh token.
+     *
+     * @param oauthProviderParams
+     * @param lastOAuthToken refresh token in lastOAuthToken *MUST* be available (not null).


You can make MUST bold. <b>MUST</b>

amitjoy · 2018-03-06T12:39:37Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthClientService.java

+     * @param lastOAuthToken refresh token in lastOAuthToken *MUST* be available (not null).
+     * @return new OAuthToken from authorization server. The new token will be saved in store with encryption.
+     * @throws IOException Web/ network issues etc. 
+     * @throws OAuthError For OAUTH error responses.  {@see https://tools.ietf.org/html/rfc6749#section-5.2}


amitjoy · 2018-03-06T12:40:11Z

...arthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthException.java

+    /**
+     *
+     */
+    private static final long serialVersionUID = -2548612391437480321L;


Empty comment blocks can be removed

amitjoy · 2018-03-06T12:40:27Z

...arthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthException.java

+    private static final long serialVersionUID = -2548612391437480321L;
+
+    public OAuthException() {
+


amitjoy · 2018-03-06T12:40:38Z

...me.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthProviderParams.java

+    /**
+     *
+     */
+    private static final long serialVersionUID = -7407238718258981852L;


empty javadoc

amitjoy · 2018-03-06T12:41:06Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+
+    /**
+     * Resource Owner Password Credentials (real users' login name)
+     * {@see https://tools.ietf.org/html/rfc6749#section-1.3.3}


amitjoy · 2018-03-06T12:41:10Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+    private String clientId;
+
+    /**
+     * Client secret {@see https://tools.ietf.org/html/rfc6749#section-2.2}


amitjoy · 2018-03-06T12:41:17Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+    /**
+     *
+     */
+    private static final long serialVersionUID = -7394989068336468235L;


empty javadoc

amitjoy · 2018-03-06T12:41:29Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthRequestParams.java

+     * This is for authorization server that uses basic authentication
+     *
+     */
+    private boolean useBasicAuthentication = false;


no need to initialize

amitjoy · 2018-04-16T07:21:11Z

....oauth2client.test/src/main/java/org/eclipse/smarthome/auth/oauth2client/test/TestAgent.java

+ * @author Gary Tse - initial contribution
+ *
+ */
+public interface TestAgent {


Since this interface is not part of public API, you can remove it and expose the implementation as a DS Component without any service interface.

It is used by ConsoleOAuthCommandExtension; refrencing both of the ResourceOwnerTestAgent and AuthorizationCodeTestAgent.

You can have an interface but it is not necessary. You can expose the implementations as @Component(service = ResourceOwnerTestAgent.class) and @Component(service = AuthorizationCodeTestAgent.class). Since the consumers reside in the same bundle, you don't need a service interface.

Thx for suggestion. If I use (service = ResourceOwnerTestAgent.class)/ (service = AuthorizationCodeTestAgent.class), do I end up only with the public methods from the 2 classes? I mean, the meat is in the abstract super class TestAgent/ AbstractTestAgent

Yes, only the public methods will be accessible if you are accessing the DS Component from a different package, otherwise you can also access the package private methods if the consumer is in the same package as the DS Components.

As discussed, leave it as it is

amitjoy · 2018-04-16T07:35:21Z

...client/src/test/java/org/eclipse/smarthome/auth/oauth2client/internal/cipher/CipherTest.java

+        return secretKey;
+    }
+
+    private static class ConfigurationAdminMock implements ConfigurationAdmin {


Instead of your own custom mock, you can use Mockito which, I believe, already exists in ESH

Thx for suggestion.

Switched; see next commit.

amitjoy · 2018-04-16T07:35:28Z

...client/src/test/java/org/eclipse/smarthome/auth/oauth2client/internal/cipher/CipherTest.java

+
+    }
+
+    private static class ConfigurationMock implements Configuration {


Instead of your own custom mock, you can use Mockito which, I believe, already exists in ESH

Thx for suggestion.

Plan to switch to Mockito?

Switched to Mockito, see next commit.

amitjoy · 2018-04-16T12:19:31Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client/build.properties

+source.. = src/main/java/
+bin.includes = META-INF/,\
+               .,\
+               OSGI-INF/org.eclipse.smarthome.auth.oauth2client.internal.OAuthFactoryImpl.xml,\


Only OSGI-INF/ will suffice

amitjoy · 2018-04-16T12:23:26Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+ * @author Gary Tse - Initial Contribution
+ *
+ */
+@Component(property = { "CIPHER_TARGET=org.eclipse.smarthome.auth.oauth2client.internal.cipher.SymmetricKeyCipher" })


You don't need the curly braces since it can be used for multiple properties.

amitjoy · 2018-04-16T12:43:53Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+                    }
+                }
+            } catch (InterruptedException e) {
+                // if lock is not acquired within the timeout or thread is interruted


Thread.currentThread.interrupt()

InterruptedExceptions should never be ignored in the code, and simply logging the exception counts in this case as "ignoring". The throwing of the InterruptedException clears the interrupted state of the Thread, so if the exception is not handled properly the fact that the thread was interrupted will be lost. Instead, InterruptedExceptions should either be rethrown - immediately or after cleaning up the method's state - or the thread should be re-interrupted by calling Thread.interrupt() even if this is supposed to be a single-threaded application. Any other course of action risks delaying thread shutdown and loses the information that the thread was interrupted - probably without finishing its task.

Similarly, the ThreadDeath exception should also be propagated. According to its JavaDoc:

If ThreadDeath is caught by a method, it is important that it be rethrown so that the thread actually dies.

kaikreuzer

Thanks for this contribution, this was quite some work!
Please find some comments below.

kaikreuzer · 2018-04-19T12:53:21Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/META-INF/MANIFEST.MF

+ org.junit;resolution:=optional,
+ org.osgi.framework,
+ org.osgi.service.cm,
+ org.osgi.service.component.annotations;resolution:=optional,


please remove, this import is not needed

I have moved junit tests here, so it should be needed. As for framework/ cm/ annotations, they are used by the test agent.

I am only talking about "org.osgi.service.component.annotations". This is not needed as the IDE & Tycho provide that already.

kaikreuzer · 2018-04-19T12:54:12Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/META-INF/MANIFEST.MF

+ org.eclipse.smarthome.io.console,
+ org.eclipse.smarthome.io.console.extensions,
+ org.hamcrest;resolution:=optional,
+ org.json,


This does not seem to be used.

Used by ConsoleOAuthCommandExtension class. The whole oauth2client.test bundle is not just junit-unit test. It acts like demo + integration + unit tests. The Console part allows the oauth client to be created/ used/ closed/ re-loaded by the console commands. See README.md

I was referring to "org.json".

Note that the comments are put UNDER the commented line by Github, not above it.

kaikreuzer · 2018-04-19T13:03:10Z

.../java/org/eclipse/smarthome/auth/oauth2client/test/console/ConsoleOAuthCommandExtension.java

+
+    public ConsoleOAuthCommandExtension() {
+        super("oauth", "Test of oauth client with console.  Available test agent: {Code, ResourceOwner}.\n"
+                + "The commands in oauth requires oauth provider data inputted in configuration admin.");


typo: inputted -> input or put in

corrected grammer: "The commands in oauth requires oauth provider data to be inputted in configuration admin."

"inputted" is no English word.

Looks ok with Cambridge dict -- https://dictionary.cambridge.org/dictionary/english/inputted

Oh, nevermind then - this surprises me and it looks weird to me (being derived from the verb "put"...)

kaikreuzer · 2018-04-19T13:07:49Z

...me.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/AccessTokenResponse.java

+
+    @Override
+    public boolean equals(Object thatAuthTokenObj) {
+


remove empty line

kaikreuzer · 2018-04-19T13:08:23Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthClientService.java

+ * This is the service factory to produce a OAuth2 service client that authenticates using OAUTH2.
+ * As this is a service factory pattern, thus the OAuthe2 service client is not shared between bundles.
+ *
+ * The basic uses of this OAuthClient is as follows:


typo: are as follows

kaikreuzer · 2018-04-19T15:16:51Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+                        PersistedParams params = gson.fromJson(value, PersistedParams.class);
+                        return params;
+                    } catch (Exception e) {
+                        logger.error("Unable to deserialize json {}, discarding PersistedParams", value);


same as above

kaikreuzer · 2018-04-19T15:17:13Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+                        LocalDateTime lastUsedDate = gson.fromJson(value, LocalDateTime.class);
+                        return lastUsedDate;
+                    } catch (Exception e) {
+                        logger.info("Unable to deserialize json {}, reset LocalDateTime to now", value);


consider json format in statement

i wish to avoid complications if the json itself cannot even be parsed for whatever reason.

kaikreuzer · 2018-04-19T15:17:30Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+            storageLock.lock();
+            try {
+                if (_storage == null) {
+                    throw new RuntimeException(EXCEPTION_STORAGE);


do not throw RuntimeException

kaikreuzer · 2018-04-19T15:17:39Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+            storageLock.lock();
+            try {
+                if (_storage == null) {
+                    throw new RuntimeException(EXCEPTION_STORAGE);


do not throw RuntimeException

kaikreuzer · 2018-04-19T15:17:53Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+                if (_storage == null) {
+                    throw new RuntimeException(EXCEPTION_STORAGE);
+                }
+


remove empty line

kaikreuzer · 2018-05-02T08:44:19Z

...2client/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthFactoryImpl.java

+     * @param oAuthStoreHandler
+     */
+    @Reference
+    protected void setoAuthStoreHandler(OAuthStoreHandler oAuthStoreHandler) {


rename to setOAuthStoreHandler

Good catch; this was generated by eclipse !

kaikreuzer · 2018-05-02T08:44:35Z

...2client/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthFactoryImpl.java

+        this.oAuthStoreHandler = oAuthStoreHandler;
+    }
+
+    protected void unsetoAuthStoreHandler(OAuthStoreHandler oAuthStoreHandler) {


rename to unsetOAuthStoreHandler

kaikreuzer · 2018-05-17T11:34:37Z

Hey @garytse, are you aware that Travis failed with test compilation errors and that the IP validation failed due to merge commits in the branch? Please fix those two issues and I'll do a final review - thanks!

kaikreuzer · 2018-06-08T09:45:47Z

@garytse As mentioned in #5142 (comment), please never do merge commits! Instead, rebase your branch on the latest master in order to have a clean PR.

kaikreuzer

Thanks @garytse, I have done a final review and left some comments - please let me know, if anything is not clear or does not make sense to you.

kaikreuzer · 2018-06-11T09:38:34Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/.project

+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.m2e.core.maven2Nature</nature>


You should not have m2e nature on any project.

fixed, sorry about that

kaikreuzer · 2018-06-11T09:39:19Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/.settings/org.eclipse.jdt.core.prefs

@@ -0,0 +1,106 @@
+eclipse.preferences.version=1


There should be no need for project specific settings, so I'd hope you can remove this file.

file removed.

kaikreuzer · 2018-06-11T09:41:40Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/META-INF/MANIFEST.MF

@@ -0,0 +1,28 @@
+Manifest-Version: 1.0
+Bundle-ManifestVersion: 2
+Bundle-Name: org.eclipse.smarthome.auth.oauth2client.test


This should be something like "Eclipse SmartHome OAuth2Client Tests"

kaikreuzer · 2018-06-11T09:42:37Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/META-INF/MANIFEST.MF

+Bundle-Version: 0.10.0.qualifier
+Bundle-Vendor: Eclipse.org/SmartHome
+Bundle-RequiredExecutionEnvironment: JavaSE-1.8
+Export-Package: org.eclipse.smarthome.auth.oauth2client.test;x-internal:=true,


Why are those packages exported?

I see; I have misunderstood the meaning of x-internal.

kaikreuzer · 2018-06-11T09:44:46Z

bundles/auth/org.eclipse.smarthome.auth.oauth2client.test/README.md

+## Try these on the OSGI console:
+
+
+smarthome.oauth Code cleanupEverything


In the (ESH default) console, the commands are smarthome oauth..., so the doc should use that format.

kaikreuzer · 2018-06-11T10:35:24Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+    protected static final int ACCESS_TOKEN_CACHE_SIZE = 50;
+
+    private static final String STORE_NAME = "StorageHandler.For.OAuthClientService";
+


remove the empty lines between the message constants

kaikreuzer · 2018-06-11T10:36:25Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+    private class StorageFacade implements AutoCloseable {
+
+        // Unusual variable name starting with underscore, _storage.
+        // This way, it cannot be confused with other storage variables locally in the enclosing class.


And obviously, you should also remove the comment.

kaikreuzer · 2018-06-11T10:38:22Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+        }
+
+        public void removeByHandle(String handle) {
+            logger.info("Removing handle {} from storage", handle);


reduce to DEBUG

kaikreuzer · 2018-06-11T10:40:12Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+    @Override
+    public void remove(@NonNull String handle) {
+        storageFacade.removeByHandle(handle);
+        // allHandles.remove(handle); // done by storage facade


line should be removed

kaikreuzer · 2018-06-11T10:42:23Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+                if (allHandles.remove(handle)) { // entry exists and successfully removed
+                    if (storage == null) {
+                        // just log the error and let it be, do not throw exception
+                        logger.warn(EXCEPTION_STORAGE);


This method is called for every handle - you wouldn't want to have so many similar WARN messages in the log file; the are not helpful. You hence should decide whether you consider this situation an illegal state (then better throw an exception to the caller) or a valid situation (then there is no need for any log message).

Well spotted. throwing IllegalStateException is consistent with the logic.

garytse · 2018-06-11T14:43:52Z

Since the change is quite a lot, I will retest the whole thing again. Will update here once completed.

kaikreuzer · 2018-06-13T14:42:42Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthClientService.java


    /**
+     * Use case 7 - get access token response. The tokens may have been retrieved previously through Use cases: 1d, 2,
+     * 3,
+     * 4, 6.


please remove the superfluous line break.

kaikreuzer

Just a few small typos and Javadoc issues that I found when looking on your latest commit.

Besides this, all looks good to me. So please let me know once your testing was successful and you have updated the remaining things - we should be good to merge then!

kaikreuzer · 2018-06-13T14:45:39Z

...smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthFactory.java

+     *
+     * @param handle the handle to the oauth service
+     * @return the oauth service, or null of the handle does not exist
+     * @throws OAuthException if the handles does not exist


Typo: "handles" -> "handle"

This actually contradicts the null return value which states "null of the handle does not exist"

kaikreuzer · 2018-06-13T14:47:01Z

...smarthome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthFactory.java

+     * Gets the oauth service for a given handle
+     *
+     * @param handle the handle to the oauth service
+     * @return the oauth service, or null of the handle does not exist


typo: of -> if

kaikreuzer · 2018-06-13T14:48:36Z

...ome.core/src/main/java/org/eclipse/smarthome/core/auth/client/oauth2/OAuthClientService.java

+     * @return An authorization URL during the Authorization Code Grant with http request parameters filled in.
+     *         e.g Produces an URL string like this:
+     *         https://oauth.provider?response_type=code&client_id=myClientId&redirect_uri=redirectURI&scope=myScope&state=mySecureRandomState
+     */


"throws" documentation is missing

kaikreuzer · 2018-06-13T14:51:29Z

.../java/org/eclipse/smarthome/auth/oauth2client/test/console/ConsoleOAuthCommandExtension.java

+    @Override
+    public List<String> getUsages() {
+        return Arrays.asList(
+                buildCommandUsage("<agent> create","create an oauth client. Agent:={Code | ResourceOwner}"),


Oh, I only saw now the Agent:={Code | ResourceOwner}. Not too readable, but ok...

kaikreuzer · 2018-06-13T14:56:00Z

...nt/src/main/java/org/eclipse/smarthome/auth/oauth2client/internal/OAuthStoreHandlerImpl.java

+    private final Set<String> allHandles = new HashSet<>(); // must be initialized
+
+    // gets replaced if Store becomes available. storageFacade handles all the times when
+    // the actual storage becomes unavilable and locking issues.


typo: unavilable

garytse · 2018-06-18T19:42:37Z

Testing completed. Many thanks for the comprehensive review and recommendations! I have learnt a lot.

kaikreuzer · 2018-06-18T21:06:05Z

Perfect, thanks!
Could you then now please squash your commits and add the other author(s) as an Also-By line to the resulting commit message?

Also-by: Kai Kreuzer <kai@openhab.org> Also-by: Michael Bock <bockm@telekom.de> Signed-off-by: Gary Tse <gary.tse@telekom.de>

maggu2810 · 2018-07-11T07:08:34Z

I have realized that the class OAuthStoreHandlerImpl uses our storage service to get a storage but does not set a classloader the storage should use.
Is this done by intention?

openhab-bot · 2018-07-14T03:14:17Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/setup-your-own-openhab-cloud-myopenhab-server-instance/24716/111

maggu2810 · 2018-07-14T08:59:17Z

@garytse Have you seen my comment?

kaikreuzer mentioned this pull request Feb 25, 2018

Add Tado AC Binding openhab/openhab-addons#2368

Closed

htreu mentioned this pull request Feb 26, 2018

[innogy] Initial implementation of innogy SmartHome binding openhab/openhab-addons#3227

Merged

kaikreuzer self-requested a review February 26, 2018 08:22

maggu2810 reviewed Mar 5, 2018

View reviewed changes

amitjoy reviewed Mar 6, 2018

View reviewed changes

amitjoy reviewed Apr 16, 2018

View reviewed changes

kaikreuzer suggested changes Apr 19, 2018

View reviewed changes

triller-telekom mentioned this pull request Apr 25, 2018

[tado] new binding openhab/openhab-addons#3279

Merged

kaikreuzer mentioned this pull request Apr 30, 2018

Initial Honeywell WiFi binding openhab/openhab-addons#339

Closed

kaikreuzer reviewed May 2, 2018

View reviewed changes

sjsf mentioned this pull request May 15, 2018

[smappee] Smappee Binding initial contribution openhab/openhab-addons#2815

Closed

kaikreuzer suggested changes Jun 11, 2018

View reviewed changes

kaikreuzer reviewed Jun 13, 2018

View reviewed changes

kaikreuzer suggested changes Jun 13, 2018

View reviewed changes

kaikreuzer approved these changes Jun 18, 2018

View reviewed changes

OAuth2 initial contribution

2c67ccb

Also-by: Kai Kreuzer <kai@openhab.org> Also-by: Michael Bock <bockm@telekom.de> Signed-off-by: Gary Tse <gary.tse@telekom.de>

kaikreuzer merged commit 032c751 into eclipse-archived:master Jun 19, 2018

garytse deleted the feature/oauth2 branch June 19, 2018 12:22

kaikreuzer mentioned this pull request Sep 18, 2018

Define OAuth client library #4551

Closed

htreu added this to the 0.10.0 milestone Oct 30, 2018

		private static final long serialVersionUID = -2548612391437480321L;

		public OAuthException() {


		}

		private static class ConfigurationMock implements Configuration {

		## Try these on the OSGI console:


		smarthome.oauth Code cleanupEverything

		protected static final int ACCESS_TOKEN_CACHE_SIZE = 50;

		private static final String STORE_NAME = "StorageHandler.For.OAuthClientService";

OAuth2 client initial contribution #5142

OAuth2 client initial contribution #5142

Conversation

garytse commented Feb 23, 2018 • edited Loading

kaikreuzer commented Feb 25, 2018

kaikreuzer commented Feb 26, 2018

kaikreuzer commented Feb 26, 2018

maggu2810 left a comment

Choose a reason for hiding this comment

garytse commented Mar 5, 2018

kaikreuzer commented Mar 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitjoy Mar 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garytse Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garytse Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikreuzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikreuzer commented May 17, 2018

kaikreuzer commented Jun 8, 2018

kaikreuzer left a comment

garytse commented Feb 23, 2018 •

edited

Loading

amitjoy Mar 6, 2018 •

edited

Loading

garytse Apr 16, 2018 •

edited

Loading

garytse Apr 16, 2018 •

edited

Loading