michielpost / Q42.HueApi

C# helper library to talk to the Philips Hue bridge
MIT License
411 stars 114 forks source link

Remote API failing on 2nd call #297

Open isaacrlevin opened 1 year ago

isaacrlevin commented 1 year ago

Hey there, I have a scenario where I am using the Remote API to change the color of my lights. It works great on first go round (getting the access token, registering the bridge and getting the API key. However on 2nd run, it gives me a 401 error.

I am saving the bridgeId and appKey in config and loading a new RemoteHueClient with those values. Looking at your code for RemoteHueClient, it looks like the HttpClient does not have the correct token set in the header. Do I need to cache the refresh or access token as well? Here is a sample of what I am doing

IRemoteAuthenticationClient authClient = new RemoteAuthenticationClient(clientId, clientSecret, appId);

authClient.BuildAuthorizeUri("sample", "consoleapp");

var webAuthenticationResult = await WebAuthenticationBroker.AuthenticateAsync(WebAuthenticationOptions.None, authorizeUri, callbackUri);

var result = authClient.ProcessAuthorizeResponse(webAuthenticationResult.ResponseData);

var accessToken = await authClient.GetToken(result.Code);

//Register app
IRemoteHueClient client = new RemoteHueClient(authClient.GetValidToken);
var bridges = await client.GetBridgesAsync();

var key = await client.RegisterAsync(bridges.First().Id, "Sample App");

//Or initialize with saved key:
client.Initialize(bridges.First().Id, "saved_key");

//Turn all lights on
var lightResult = await client.SendCommandAsync(new LightCommand().TurnOn());

// To this point it all works, however if I save bridgeId.First().Id and key to a file and load them at runtime like this, it fails

if (_client == null || !_client.IsInitialized)
{
  _client = new RemoteHueClient(_authClient.GetValidToken);
  _client.Initialize("savedbridgeId", "savedAppKey");
}
// this line will return a 401, I assume because authClient doesn't have a token in it
var lights = await _client.GetLightsAsync();

If I am doing something wrong let me know

michielpost commented 1 year ago

What kind of app are you building? UWP? Last time I checked my UWP Remote sample didn't run on my PC anymore. So I would need to fix that first to be able to properly test it.

But looking at the code, it gives a few hints. Your code:

if (_client == null || !_client.IsInitialized)
{
  _client = new RemoteHueClient(_authClient.GetValidToken);
  _client.Initialize("savedbridgeId", "savedAppKey");
}

But according to the sample, it should be something like this: https://github.com/michielpost/Q42.HueApi/blob/master/src/Q42.HueApi.RemoteApi.Sample/MainPage.xaml.cs#LL51C5-L53C79

You should initialize the RemoteAuthenticationClient. You need an AccessTokenResponse for that, in your code you got that from the authClient.GetToken(result.Code); call.

if (_client == null || !_client.IsInitialized)
{
  authClient.Initialize(accessToken);
  _client = new RemoteHueClient(authClient.GetValidToken);
  _client.Initialize("savedbridgeId", "savedAppKey");
}

I hope this helps!

isaacrlevin commented 1 year ago

It's a WPF app. I think my point is how do I Initialize a new RemoteHueClient WITHOUT doing the Auth dance? In my case I want to have the user Auth and cache their api key

michielpost commented 1 year ago

This should initialize a RemoteHueClient without the web popup and authorization if you stored the accessToken object:

IRemoteAuthenticationClient authClient = new RemoteAuthenticationClient(clientId, clientSecret, appId);
AccessTokenResponse storedAccessToken = //TODO: Get this from storage somewhere
authClient.Initialize(storedAccessToken);

IRemoteHueClient client = new RemoteHueClient(authClient.GetValidToken);
isaacrlevin commented 1 year ago

I gave that a try, and I think there is a bigger issue here. You have a GetHttpClient method in RemoteHueClient tha hydrates the headers with the access token. This is not fired UNLESS you call GetBridgesAsync so the idea that you cache the token and the bridge, it fails. If you agree, I can submit a PR to fix it.

michielpost commented 1 year ago

Yes the idea is to cache the full AccessTokenResponse object.

I think I see what you mean, the GetHttpClient is also called from all the other methods in the base HueClient. The methods in the base HueClient call GetHttpClient and should use the new implementation in the RemoteHueClient and add the headers with the access token that way. But I see that's not working as expected.

I fixed it by making the original GetHttpClient virtual. It's released on NuGet in version 3.21.0. Let me know if that works for you. Otherwise a PR is also welcome.