From 9c661e2e4b165d4d96dd08ab7dad72ca87e9ce92 Mon Sep 17 00:00:00 2001 From: Adrian Kumpf <8999358+adriankumpf@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:57:12 +0200 Subject: [PATCH 1/2] Add HTTP Basic auth as an alternative to OAuth Every endpoint accepts the account's email and password in an ordinary Authorization header, which sidesteps the token lifecycle entirely: nothing expires, there is no consumer to register, and neither of the per-IP rate limits on consumer_token and authorize applies. Verified against meters, field_names, last_reading, readings, statistics, devices, disaggregation and activities. The 401 for a missing credential advertises the scheme itself: www-authenticate: Basic realm="Discovergy", charset="UTF-8" The API documents none of this, so basic_auth/3 is an alternative to login/3 rather than a replacement, and login/3 stays the documented way in. A wrong password comes back as a 401 that says so, unlike the empty-bodied 401 of an expired token, so the two never need telling apart. Nothing goes out when the client is built, so basic_auth/3 returns the client rather than {:ok, client}. The :consumer and :token fields collapse into a single :credentials field holding either an OAuth session or the Basic auth pair. Only two of the four combinations were ever legal, and the request path now dispatches on one value instead of picking its way through them. The client no longer knows how either scheme builds a header: OAuth.authorization/4 and BasicAuth.authorization/1 do that themselves, both returning a lowercase name to match the rest of the request. credentials/1 reads the session back for persistence, which the struct being opaque had previously left to reaching into documented fields. Reauthorizing a Basic auth client reports :not_logged_in, since there is no OAuth session to renew. --- CHANGELOG.md | 13 ++- README.md | 9 ++- guides/api-quirks.md | 49 +++++++++--- lib/discovergy.ex | 9 ++- lib/discovergy/basic_auth.ex | 14 ++++ lib/discovergy/client.ex | 136 +++++++++++++++++++++----------- lib/discovergy/error.ex | 3 +- lib/discovergy/oauth.ex | 58 ++++++++++---- test/discovergy/client_test.exs | 69 +++++++++++++--- test/discovergy/oauth_test.exs | 85 +++++++++----------- test/support/discovergy_case.ex | 13 ++- 11 files changed, 315 insertions(+), 143 deletions(-) create mode 100644 lib/discovergy/basic_auth.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index cb22b1e..a8c309e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,14 @@ - Raise on an option the endpoint does not have. A misspelled `:resolution` or `:fields` used to be dropped silently, and the request went out without it. - `Discovergy.WebsiteAccessCode.generate/2` returns the access code as the API sends it. It used to be decoded as a query string, and the first key of the resulting map was returned as the code. - An unsuccessful response with an empty body is reported as `{:http_error, status}` instead of `:unknown`, which rendered as `":unknown"`. +- Replace the `:consumer` and `:token` fields of `Discovergy.Client` with a single `:credentials` field, read with `Discovergy.Client.credentials/1` and restored through the `:credentials` option of `Discovergy.Client.new/1`. A client authenticates one way or the other, and holding the pieces of an OAuth session in loose fields left no room for anything else. The struct is opaque, so the accessor also gets persistence out of the business of reaching into it. + + ```diff + - # persist client.consumer and client.token, then later: + - client = Discovergy.Client.new(consumer: consumer, token: token) + + # persist Discovergy.Client.credentials(client), then later: + + client = Discovergy.Client.new(credentials: credentials) + ``` ### Bug Fixes @@ -32,11 +40,12 @@ ### Security -- Redact the OAuth secrets from `Discovergy.Client`, `Discovergy.OAuth.Consumer` and `Discovergy.OAuth.Token` when they are inspected, so a Logger metadata field or a crash report no longer prints the credentials of the session. +- Redact the credentials of the session from `Discovergy.Client` and the structs it holds when they are inspected, so a Logger metadata field or a crash report no longer prints the OAuth secrets or the Basic auth password. ### Changes -- Document the quirks of the API: that HTTP Basic auth works and avoids the token lifecycle entirely, token expiry, the rate limits on consumer registration and authorization, the shape its errors arrive in, and the undocumented meter fields. +- Add `Discovergy.Client.basic_auth/3`, which authenticates with HTTP Basic auth instead of OAuth. Every endpoint accepts the account's email and password directly, so there is no token to expire, no consumer to register and neither of the rate limits `login/3` runs into, and no request is spent signing in. The API documents none of it, which is why this is an alternative to `login/3` rather than a replacement. +- Document the quirks of the API: token expiry, the rate limits on consumer registration and authorization, the shape its errors arrive in, and the undocumented meter fields. - Add `Discovergy.Client.reauthorize/3`, which obtains a new access token while reusing the consumer registered by `login/3`. The API rate limits `consumer_token` requests and asks clients to reuse tokens, so an application that refreshed by calling `login/3` again would eventually be answered with a `429`. - Add the `kwh_scaling_factor`, `printed_full_serial_number`, `storage_numbers` and `submeter` fields to `Discovergy.Meter`. The API returns them but they were silently dropped. - Fix the `Discovergy.Measurement` typespec: `values` is a map, not a list of maps. diff --git a/README.md b/README.md index 09a78d2..4de6952 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,13 @@ iex> {:ok, client} = Discovergy.Client.reauthorize(client, email, password) The API rate limits consumer registrations, so an application that renews its token by logging in again is eventually turned away with a `429`. +The API also accepts plain HTTP Basic auth, which `Discovergy.Client.basic_auth/3` sends. It skips the token lifecycle and the rate limits entirely, at the price of being undocumented: + +```elixir +iex> client = Discovergy.Client.new() |> Discovergy.Client.basic_auth(email, password) +#Discovergy.Client +``` + Then pass the `client` to the respective endpoint function. For example, to list all meters the user has access to: ```elixir @@ -92,7 +99,7 @@ iex> Discovergy.Measurements.get_last_reading(client, "c1972a89ce3a4d58aadcb7908 ## Quirks of the API -The API behaves in ways its [official documentation](https://api.inexogy.com/docs/) does not cover: access tokens expire and come back as a `401` with an empty body, consumer registration and authorization are rate limited per IP, upstream errors arrive as HTML, and `/meters` returns fields that are documented nowhere. Plain HTTP Basic auth also works on every data endpoint, undocumented, which avoids the token lifecycle entirely. +The API behaves in ways its [official documentation](https://api.inexogy.com/docs/) does not cover: access tokens expire and come back as a `401` with an empty body, consumer registration and authorization are rate limited per IP, upstream errors arrive as HTML, and `/meters` returns fields that are documented nowhere. The Basic auth above is undocumented too. [Quirks of the API](guides/api-quirks.md) collects what running against it turned up. diff --git a/guides/api-quirks.md b/guides/api-quirks.md index c28f08a..158ef9b 100644 --- a/guides/api-quirks.md +++ b/guides/api-quirks.md @@ -6,27 +6,47 @@ running against it. Worth reading before deploying anything long-lived. ## HTTP Basic auth works, and is not documented anywhere -Every data endpoint accepts plain HTTP Basic auth with the account's email and +Every endpoint accepts plain HTTP Basic auth with the account's email and password: ``` curl -u 'demo@inexogy.com:demo' https://api.inexogy.com/public/v1/meters ``` -Verified against `meters`, `field_names`, `last_reading`, `devices` and -`statistics`, all returning `200`. Without credentials, or with a wrong -password, the same request is a `401`. +Verified against `meters`, `field_names`, `last_reading`, `readings`, +`statistics`, `devices`, `disaggregation` and `activities`, all returning +`200`. Without credentials the same request is a `401`, and the challenge it +comes back with names the scheme: + +``` +www-authenticate: Basic realm="Discovergy", charset="UTF-8" +``` This matters because it sidesteps everything below about tokens: nothing to expire, no consumer to register, and neither of the rate limits. The [ioBroker adapter](https://github.com/DrozmotiX/ioBroker.discovergy) has used it exclusively for years. -This library uses OAuth 1.0a, which the official documentation describes as the -way in, and exposes no way to send Basic auth instead. Basic auth is -undocumented, so it carries the risk that anything undocumented does: it could -be withdrawn without notice. Everything below applies whenever OAuth is used, -which here is always. +`Discovergy.Client.basic_auth/3` sends it: + +```elixir +client = Discovergy.Client.new() |> Discovergy.Client.basic_auth(email, password) +``` + +Nothing goes out when you call it, so a wrong password first shows up on the +next request, as a `401` that does say what is wrong: + +``` +401 Unauthorized: Invalid email or password supplied in the HTTP Authorization header +``` + +That is the whole difference in error handling: an expired OAuth token is an +empty-bodied `401` to be recovered from, while a `401` here is a credential +that will not start working on its own. + +Basic auth is undocumented, so it carries the risk that anything undocumented +does: it could be withdrawn without notice. `Discovergy.Client.login/3` remains +the documented way in, and everything below applies whenever it is used. ## A public demo account exists @@ -144,19 +164,22 @@ accounts does not lift them. Use `Discovergy.Client.reauthorize/3` rather than `Discovergy.Client.login/3` to renew a token. It reuses the consumer registered by the first login and never -calls `consumer_token`. The consumer can also be persisted and handed back to -`Discovergy.Client.new/1` so it survives a restart: +calls `consumer_token`. The session can also be persisted and handed back to +`Discovergy.Client.new/1` so it survives a restart, spending neither call: ```elixir {:ok, client} = Discovergy.Client.new() |> Discovergy.Client.login(email, password) -# persist client.consumer and client.token, then later: -client = Discovergy.Client.new(consumer: consumer, token: token) +# persist Discovergy.Client.credentials(client), then later: +client = Discovergy.Client.new(credentials: credentials) ``` A persisted consumer expires like any other, so be ready for the `:consumer_rejected` above and register a new one. +Basic auth is subject to neither limit, which is the other reason to reach for +it. + `authorize` is limited more tightly than `consumer_token`. Two calls in quick succession from one address are enough to trigger it. diff --git a/lib/discovergy.ex b/lib/discovergy.ex index b8d4795..6a5a8b3 100644 --- a/lib/discovergy.ex +++ b/lib/discovergy.ex @@ -13,9 +13,12 @@ defmodule Discovergy do Access tokens expire. Use `Discovergy.Client.reauthorize/3` to get a new one rather than logging in again, so the consumer registered by `login/3` is reused. Consumer registration is rate limited per IP, so an application that - renews its token by logging in again is eventually answered with a `429`. See - [Quirks of the API](api-quirks.md) for the rest of what running against it - turned up. + renews its token by logging in again is eventually answered with a `429`. + + The API also accepts HTTP Basic auth, which `Discovergy.Client.basic_auth/3` + sends. It has no token to expire and no rate limit to run into, but it is + undocumented. See [Quirks of the API](api-quirks.md) for that and the rest of + what running against the API turned up. Then pass the `client` to the respective endpoint function. For example, to list all meters the user has access to: diff --git a/lib/discovergy/basic_auth.ex b/lib/discovergy/basic_auth.ex new file mode 100644 index 0000000..c53a093 --- /dev/null +++ b/lib/discovergy/basic_auth.ex @@ -0,0 +1,14 @@ +defmodule Discovergy.BasicAuth do + @moduledoc false + + @type t :: %__MODULE__{email: String.t(), password: String.t()} + + @derive {Inspect, except: [:password]} + @enforce_keys [:email, :password] + defstruct [:email, :password] + + @spec authorization(t) :: {String.t(), String.t()} + def authorization(%__MODULE__{email: email, password: password}) do + {"authorization", "Basic " <> Base.encode64("#{email}:#{password}")} + end +end diff --git a/lib/discovergy/client.ex b/lib/discovergy/client.ex index 2439c57..f4e85b5 100644 --- a/lib/discovergy/client.ex +++ b/lib/discovergy/client.ex @@ -2,36 +2,41 @@ defmodule Discovergy.Client do @moduledoc """ A Discovergy API Client - Access tokens expire, and an expired one comes back as a `401` with an empty - body. Use `reauthorize/3` rather than `login/3` to get a new one: the API - rate limits consumer registration and asks clients to reuse tokens. + There are two ways to authenticate, and a client uses one of them for its + lifetime: - Consumers expire too. `reauthorize/3` then fails with - `reason: :consumer_rejected`, and `login/3` registers a new one. + - `login/3` runs the OAuth 1.0a flow the [official + documentation](https://api.inexogy.com/docs/) describes. Both the access + token and the consumer behind it expire, so a long-running client renews + them with `reauthorize/3` and, when that is refused, `login/3`. + + - `basic_auth/3` sends the email and password with every request. Nothing + expires and nothing is rate limited, but the API documents no such thing. See [Quirks of the API](api-quirks.md) for the behaviour this library has to work around. """ - alias Discovergy.{Config, Error, OAuth} + alias Discovergy.{BasicAuth, Config, Error, OAuth} @base_url "https://api.inexogy.com/public/v1" @user_agent "github.com/adriankumpf/discovergy" @form_urlencoded "application/x-www-form-urlencoded" + @opaque credentials :: OAuth.t() | BasicAuth.t() + @opaque t :: %__MODULE__{ base_url: String.t(), http_client: module, - consumer: OAuth.Consumer.t() | nil, - token: OAuth.Token.t() | nil + credentials: credentials | nil } - # The client carries the OAuth secrets of the session. Keep them out of - # logs, crash reports and iex output. + # The client carries the credentials of the session. Keep them out of logs, + # crash reports and iex output. @derive {Inspect, only: [:base_url]} @enforce_keys [:base_url, :http_client] - defstruct [:base_url, :http_client, :consumer, :token] + defstruct [:base_url, :http_client, :credentials] @doc """ Creates a new Discovergy API client. @@ -41,15 +46,14 @@ defmodule Discovergy.Client do - `:base_url` - the base URL for all endpoints (default: `#{@base_url}`) - `:http_client` - a module implementing the `Discovergy.HTTPClient` behaviour (default: the `:client` application environment setting) - - `:consumer` - the consumer of a previous session, taken from - `client.consumer` after a `login/3` - - `:token` - the access token of a previous session, taken from - `client.token` + - `:credentials` - the credentials of a previous session, as `credentials/1` + returns them - Passing `:consumer` and `:token` restores a session that was persisted - elsewhere, so a restart neither registers another consumer nor spends an - `authorize` call. Both are rate limited per IP. The client is then usable - right away, and `reauthorize/3` works on it once the token expires. + Passing `:credentials` restores a session that was persisted elsewhere. For + an OAuth session that means a restart neither registers another consumer nor + spends an `authorize` call, both of which are rate limited per IP. The client + is then usable right away, and `reauthorize/3` works on it once the token + expires. Basic auth has nothing worth restoring; call `basic_auth/3` instead. ## Examples @@ -62,15 +66,40 @@ defmodule Discovergy.Client do %__MODULE__{ base_url: opts[:base_url] || @base_url, http_client: opts[:http_client] || Config.client(), - consumer: opts[:consumer], - token: opts[:token] + credentials: opts[:credentials] } end + @doc """ + Returns the credentials of the session, or `nil` if the client has none. + + Worth persisting for an OAuth session, which `new/1` takes back to skip the + two rate limited calls `login/3` would spend after a restart. Serialize it + with `:erlang.term_to_binary/1`, since the structs redact themselves when + inspected and encode to no other format, and be ready for a blob written by + an older version of this library not to load. + + A Basic auth session is worth nothing persisted: `basic_auth/3` rebuilds it + from the same email and password, and storing it writes the account's + password somewhere new. + + ## Examples + + iex> Discovergy.Client.new() |> Discovergy.Client.credentials() + nil + + """ + @spec credentials(t) :: credentials | nil + def credentials(%__MODULE__{credentials: credentials}), do: credentials + @doc """ Authenticate with the Discovergy API using the email address and password of the user. + This runs the OAuth 1.0a flow: it registers a consumer and exchanges the + credentials for an access token, both of which expire. Use `reauthorize/3` to + renew the token, or `basic_auth/3` to avoid the lifecycle altogether. + ## Examples iex> {:ok, client} = Discovergy.Client.new() @@ -81,8 +110,8 @@ defmodule Discovergy.Client do @spec login(t, String.t(), String.t()) :: {:ok, t} | {:error, Error.t()} def login(%__MODULE__{} = client, email, password) when is_binary(email) and is_binary(password) do - with {:ok, {consumer, token}} <- OAuth.login(client, email, password) do - {:ok, %__MODULE__{client | token: token, consumer: consumer}} + with {:ok, oauth} <- OAuth.login(client, email, password) do + {:ok, %__MODULE__{client | credentials: oauth}} end end @@ -101,6 +130,9 @@ defmodule Discovergy.Client do Named for what it does: the API has no credential-free refresh, so this needs the user's password just as `login/3` does. Only the consumer is spared. + A client that has no OAuth session, because it never logged in or because it + uses `basic_auth/3`, fails with `reason: :not_logged_in`. + ## When the consumer is gone The API drops consumers too, usually at the nightly maintenance that expires @@ -126,16 +158,40 @@ defmodule Discovergy.Client do """ @spec reauthorize(t, String.t(), String.t()) :: {:ok, t} | {:error, Error.t()} - def reauthorize(%__MODULE__{consumer: nil}, email, password) + def reauthorize(%__MODULE__{credentials: %OAuth{consumer: consumer}} = client, email, password) + when is_binary(email) and is_binary(password) do + with {:ok, oauth} <- OAuth.reauthorize(client, consumer, email, password) do + {:ok, %__MODULE__{client | credentials: oauth}} + end + end + + def reauthorize(%__MODULE__{}, email, password) when is_binary(email) and is_binary(password) do {:error, %Error{reason: :not_logged_in}} end - def reauthorize(%__MODULE__{consumer: consumer} = client, email, password) + @doc """ + Authenticate with HTTP Basic auth, using the email address and password of + the user. + + Every endpoint accepts the credentials directly, so there is no token to + expire, no consumer to register and neither of the rate limits `login/3` runs + into. Nothing is sent here: the client is ready to use, and a wrong password + surfaces as a `401` on the first request. + + The API documents none of this, so it could be withdrawn without notice. + `login/3` is the documented way in. + + ## Examples + + iex> Discovergy.Client.new() |> Discovergy.Client.basic_auth(email, password) + #Discovergy.Client + + """ + @spec basic_auth(t, String.t(), String.t()) :: t + def basic_auth(%__MODULE__{} = client, email, password) when is_binary(email) and is_binary(password) do - with {:ok, token} <- OAuth.reauthorize(client, consumer, email, password) do - {:ok, %__MODULE__{client | token: token}} - end + %__MODULE__{client | credentials: %BasicAuth{email: email, password: password}} end @doc false @@ -152,11 +208,10 @@ defmodule Discovergy.Client do defp request(%__MODULE__{} = client, method, path, body, opts) do url = build_url(client.base_url, path, opts[:query] || []) - consumer = Keyword.get(opts, :consumer, client.consumer) - token = Keyword.get(opts, :token, client.token) + credentials = Keyword.get(opts, :credentials, client.credentials) headers = - sign(method, url, body, consumer, token) ++ + authorization(credentials, method, url, body) ++ [{"user-agent", @user_agent} | content_type(method)] client.http_client.request( @@ -174,22 +229,13 @@ defmodule Discovergy.Client do defp content_type(:post), do: [{"content-type", @form_urlencoded}] defp content_type(_method), do: [] - defp sign(_method, _url, _body, nil = _consumer, _token), do: [] - - defp sign(method, url, body, consumer, token) do - credentials = - OAuther.credentials( - consumer_key: consumer.key, - consumer_secret: consumer.secret, - token: token && token.oauth_token, - token_secret: token && token.oauth_token_secret - ) + defp authorization(nil, _method, _url, _body), do: [] - {header, _req_params} = - OAuther.sign(to_string(method), url, body, credentials) |> OAuther.header() + defp authorization(%BasicAuth{} = basic, _method, _url, _body), + do: [BasicAuth.authorization(basic)] - [header] - end + defp authorization(%OAuth{} = oauth, method, url, body), + do: [OAuth.authorization(oauth, method, url, body)] # Optional parameters are passed as nil rather than dropped at every call # site, because the API rejects the ones it does not expect to be empty. diff --git a/lib/discovergy/error.ex b/lib/discovergy/error.ex index 3bc7cba..ea394a0 100644 --- a/lib/discovergy/error.ex +++ b/lib/discovergy/error.ex @@ -10,7 +10,8 @@ defmodule Discovergy.Error do Two reasons come from this library rather than from the API: - `:not_logged_in` - `Discovergy.Client.reauthorize/3` was called on a client - that has no consumer, so there is nothing to reuse. + with no OAuth session to renew: it never logged in, or it authenticates + with `Discovergy.Client.basic_auth/3`, which has nothing that expires. - `:consumer_rejected` - the API no longer accepts the consumer of the client being reauthorized. Register a new one with `Discovergy.Client.login/3`. """ diff --git a/lib/discovergy/oauth.ex b/lib/discovergy/oauth.ex index 3b5447e..12b15a2 100644 --- a/lib/discovergy/oauth.ex +++ b/lib/discovergy/oauth.ex @@ -40,43 +40,69 @@ defmodule Discovergy.OAuth do def into(attrs), do: Model.cast(__MODULE__, attrs) end + @type t() :: %__MODULE__{consumer: Consumer.t(), token: Token.t() | nil} + + @enforce_keys [:consumer] + defstruct [:consumer, :token] + @doc """ Runs the four steps of the [OAuth 1.0 flow](https://tools.ietf.org/html/rfc5849): register the client application, obtain a request token, authorize it with the user's credentials and exchange it for an access token. """ - @spec login(Client.t(), String.t(), String.t()) :: - {:ok, {Consumer.t(), Token.t()}} | {:error, Error.t()} + @spec login(Client.t(), String.t(), String.t()) :: {:ok, t()} | {:error, Error.t()} def login(%Client{} = client, email, password) do - with {:ok, consumer} <- register_consumer(client), - {:ok, access_token} <- reauthorize(client, consumer, email, password) do - {:ok, {consumer, access_token}} + with {:ok, consumer} <- register_consumer(client) do + reauthorize(client, consumer, email, password) end end @doc """ - Steps 2 to 4, for a consumer that is already registered. + Steps 2 to 4, for a consumer that is already registered. Taking the consumer + rather than the whole session is the point of the function: it is all that is + reused. """ @spec reauthorize(Client.t(), Consumer.t(), String.t(), String.t()) :: - {:ok, Token.t()} | {:error, Error.t()} + {:ok, t()} | {:error, Error.t()} def reauthorize(%Client{} = client, %Consumer{} = consumer, email, password) do with {:ok, request_token} <- get_request_token(client, consumer), - {:ok, verifier} <- authorize(client, request_token, email, password) do - get_access_token(client, consumer, request_token, verifier) + {:ok, verifier} <- authorize(client, request_token, email, password), + {:ok, token} <- get_access_token(client, consumer, request_token, verifier) do + {:ok, %__MODULE__{consumer: consumer, token: token}} end end + @spec authorization(t(), atom(), String.t(), keyword()) :: {String.t(), String.t()} + def authorization(%__MODULE__{consumer: consumer, token: token}, method, url, body) do + credentials = + OAuther.credentials( + consumer_key: consumer.key, + consumer_secret: consumer.secret, + token: token && token.oauth_token, + token_secret: token && token.oauth_token_secret + ) + + # OAuther names the header "Authorization"; the rest of the request uses + # lowercase names, and HTTP does not care which. + {{_name, value}, _req_params} = + method |> to_string() |> OAuther.sign(url, body, credentials) |> OAuther.header() + + {"authorization", value} + end + defp register_consumer(client) do - opts = [consumer: nil, token: nil] + body = [{"client", @client_id}] with {:ok, consumer} <- - Client.post(client, "/oauth1/consumer_token", [{"client", @client_id}], opts) do + Client.post(client, "/oauth1/consumer_token", body, credentials: nil) do {:ok, Consumer.into(consumer)} end end defp get_request_token(client, consumer) do - case Client.post(client, "/oauth1/request_token", [], consumer: consumer, token: nil) do + credentials = %__MODULE__{consumer: consumer} + + case Client.post(client, "/oauth1/request_token", [], credentials: credentials) do {:ok, body} -> {:ok, Token.into(URI.decode_query(body))} @@ -93,9 +119,8 @@ defmodule Discovergy.OAuth do defp authorize(client, request_token, email, password) do query = [email: email, password: password, oauth_token: request_token.oauth_token] - opts = [query: query, consumer: nil, token: nil] - with {:ok, body} <- Client.get(client, "/oauth1/authorize", opts) do + with {:ok, body} <- Client.get(client, "/oauth1/authorize", query: query, credentials: nil) do %{"oauth_verifier" => verifier} = URI.decode_query(body) {:ok, verifier} end @@ -103,9 +128,10 @@ defmodule Discovergy.OAuth do defp get_access_token(client, consumer, request_token, verifier) do body = [{"oauth_verifier", verifier}] - opts = [consumer: consumer, token: request_token] + credentials = %__MODULE__{consumer: consumer, token: request_token} - with {:ok, response_body} <- Client.post(client, "/oauth1/access_token", body, opts) do + with {:ok, response_body} <- + Client.post(client, "/oauth1/access_token", body, credentials: credentials) do {:ok, Token.into(URI.decode_query(response_body))} end end diff --git a/test/discovergy/client_test.exs b/test/discovergy/client_test.exs index cd468f7..d3cd419 100644 --- a/test/discovergy/client_test.exs +++ b/test/discovergy/client_test.exs @@ -12,12 +12,12 @@ defmodule Discovergy.ClientTest do end) end - test "identifies itself and does not sign requests when logged out", %{client: client} do + test "identifies itself and does not authenticate requests when logged out", %{client: client} do assert {:ok, []} = Client.get(client, "/meters") assert_receive {:request, %{headers: headers, body: ""}} assert {"user-agent", "github.com/adriankumpf/discovergy"} in headers - refute List.keyfind(headers, "Authorization", 0) + assert authorization(headers) == nil refute List.keyfind(headers, "content-type", 0) end @@ -33,24 +33,71 @@ defmodule Discovergy.ClientTest do assert {:ok, []} = Client.get(client, "/meters") assert_receive {:request, %{headers: headers}} - assert {"Authorization", "OAuth " <> params} = List.keyfind(headers, "Authorization", 0) + assert "OAuth " <> params = authorization(headers) assert params =~ ~s(oauth_consumer_key="%24key") assert params =~ ~s(oauth_token="%24access_token") assert params =~ "oauth_signature=" end - @tag :logged_in - test "keeps the credentials out of the inspected client", %{client: client} do - for inspected <- [inspect(client), inspect(client.consumer), inspect(client.token)] do - refute inspected =~ client.consumer.secret - refute inspected =~ client.token.oauth_token_secret - end - end - test "talks to the configured base URL" do client = Client.new(base_url: "http://localhost:4000/v1", http_client: TestClient) assert {:ok, []} = Client.get(client, "/meters") assert_receive {:request, %{url: "http://localhost:4000/v1/meters"}} end + + describe "basic_auth/3" do + setup %{client: client} do + {:ok, client: Client.basic_auth(client, "demo@inexogy.com", "demo")} + end + + test "sends the credentials with every request", %{client: client} do + assert {:ok, []} = Client.get(client, "/meters") + + assert_receive {:request, %{headers: headers}} + assert authorization(headers) == "Basic " <> Base.encode64("demo@inexogy.com:demo") + end + + test "sends nothing to establish the session" do + refute_receive {:request, _} + end + + test "has nothing to reauthorize", %{client: client} do + assert {:error, %Discovergy.Error{reason: :not_logged_in}} = + Client.reauthorize(client, "demo@inexogy.com", "demo") + end + end + + describe "credentials/1" do + test "are absent until the client authenticates", %{client: client} do + assert Client.credentials(client) == nil + end + + @tag :logged_in + test "restore a session into a new client", %{client: client} do + restored = Client.new(http_client: TestClient, credentials: Client.credentials(client)) + + assert {:ok, []} = Client.get(restored, "/meters") + assert_receive {:request, %{headers: headers}} + assert authorization(headers) =~ ~s(oauth_token="%24access_token") + end + end + + describe "inspect" do + @tag :logged_in + test "keeps the OAuth secrets out of the output", %{client: client} do + for inspected <- [inspect(client), inspect(Client.credentials(client))] do + refute inspected =~ "$secret" + refute inspected =~ "$access_token_secret" + end + end + + test "keeps the password out of the output", %{client: client} do + client = Client.basic_auth(client, "demo@inexogy.com", "hunter2") + + for inspected <- [inspect(client), inspect(Client.credentials(client))] do + refute inspected =~ "hunter2" + end + end + end end diff --git a/test/discovergy/oauth_test.exs b/test/discovergy/oauth_test.exs index c06d20f..43a5a15 100644 --- a/test/discovergy/oauth_test.exs +++ b/test/discovergy/oauth_test.exs @@ -1,42 +1,37 @@ defmodule Discovergy.OAuthTest do use Discovergy.Case, async: true + alias Discovergy.{Client, OAuth} + test "login", %{client: client} do mock(&full_authorization/1) - assert {:ok, %Discovergy.Client{consumer: consumer, token: token}} = - Discovergy.Client.login(client, "$email", "$password") - - assert %Discovergy.OAuth.Consumer{ - attributes: %{}, - key: "$key", - owner: "$client_id", - principal: nil, - secret: "$secret" - } == consumer - - assert %Discovergy.OAuth.Token{ - oauth_token: "$access_token", - oauth_token_secret: "$access_token_secret" - } == token + assert {:ok, client} = Client.login(client, "$email", "$password") + + assert %OAuth{ + consumer: %OAuth.Consumer{ + attributes: %{}, + key: "$key", + owner: "$client_id", + principal: nil, + secret: "$secret" + }, + token: %OAuth.Token{ + oauth_token: "$access_token", + oauth_token_secret: "$access_token_secret" + } + } == Client.credentials(client) end - test "does not reuse the consumer", %{client: client} do + test "does not reuse the consumer" do mock(&full_authorization/1) - consumer = %Discovergy.OAuth.Consumer{ - attributes: %{}, - key: "$key", - owner: "DiscoX", - principal: nil, - secret: "$secret" - } - - assert {:ok, %Discovergy.Client{consumer: new_consumer}} = - put_in(client.consumer, consumer) - |> Discovergy.Client.login("$email", "$password") + stale = %OAuth{consumer: %OAuth.Consumer{key: "$stale_key", secret: "$stale_secret"}} + client = Client.new(http_client: TestClient, credentials: stale) - assert new_consumer != consumer + assert {:ok, client} = Client.login(client, "$email", "$password") + assert %OAuth{consumer: consumer} = Client.credentials(client) + assert consumer != stale.consumer end @tag :logged_in @@ -48,7 +43,7 @@ defmodule Discovergy.OAuthTest do full_authorization(response) end) - assert {:ok, %Discovergy.Client{}} = Discovergy.Client.login(client, "$email", "$password") + assert {:ok, %Client{}} = Client.login(client, "$email", "$password") # These two open the flow, so there is nothing to sign them with yet. assert_receive {"/public/v1/oauth1/consumer_token", nil} @@ -64,15 +59,16 @@ defmodule Discovergy.OAuthTest do full_authorization(response) end) - assert {:ok, %Discovergy.Client{consumer: consumer, token: token}} = - Discovergy.Client.reauthorize(client, "$email", "$password") - - assert consumer == client.consumer + assert %OAuth{consumer: consumer} = Client.credentials(client) + assert {:ok, renewed} = Client.reauthorize(client, "$email", "$password") - assert %Discovergy.OAuth.Token{ - oauth_token: "$access_token", - oauth_token_secret: "$access_token_secret" - } == token + assert %OAuth{ + consumer: ^consumer, + token: %OAuth.Token{ + oauth_token: "$access_token", + oauth_token_secret: "$access_token_secret" + } + } = Client.credentials(renewed) assert_receive {:path, "/public/v1/oauth1/request_token"} assert_receive {:path, "/public/v1/oauth1/authorize"} @@ -91,8 +87,7 @@ defmodule Discovergy.OAuthTest do full_authorization(response) end) - assert {:ok, %Discovergy.Client{}} = - Discovergy.Client.reauthorize(client, "$email", "$password") + assert {:ok, %Client{}} = Client.reauthorize(client, "$email", "$password") # Signed in already, but this one still has to go out unsigned. assert_receive {"/public/v1/oauth1/authorize", nil} @@ -112,7 +107,7 @@ defmodule Discovergy.OAuthTest do assert {:error, %Discovergy.Error{reason: :consumer_rejected, response: {unquote(status), [], ""}}} = - Discovergy.Client.reauthorize(client, "$email", "$password") + Client.reauthorize(client, "$email", "$password") end end @@ -129,19 +124,13 @@ defmodule Discovergy.OAuthTest do end) assert {:error, %Discovergy.Error{reason: unquote(body)}} = - Discovergy.Client.reauthorize(client, "$email", "$password") + Client.reauthorize(client, "$email", "$password") end end test "refuses to reauthorize a client that is not signed in", %{client: client} do assert {:error, %Discovergy.Error{reason: :not_logged_in}} = - Discovergy.Client.reauthorize(client, "$email", "$password") - end - - defp authorization(headers) do - Enum.find_value(headers, fn {key, value} -> - if String.downcase(key) == "authorization", do: value - end) + Client.reauthorize(client, "$email", "$password") end # Replies to each of the four steps of the flow. A step that sends something diff --git a/test/support/discovergy_case.ex b/test/support/discovergy_case.ex index a1e4486..b2359e4 100644 --- a/test/support/discovergy_case.ex +++ b/test/support/discovergy_case.ex @@ -1,10 +1,13 @@ defmodule Discovergy.Case do use ExUnit.CaseTemplate + alias Discovergy.OAuth alias Discovergy.OAuth.{Consumer, Token} - @consumer %Consumer{attributes: %{}, key: "$key", owner: "$client_id", secret: "$secret"} - @token %Token{oauth_token: "$access_token", oauth_token_secret: "$access_token_secret"} + @oauth %OAuth{ + consumer: %Consumer{attributes: %{}, key: "$key", owner: "$client_id", secret: "$secret"}, + token: %Token{oauth_token: "$access_token", oauth_token_secret: "$access_token_secret"} + } using do quote do @@ -15,7 +18,7 @@ defmodule Discovergy.Case do # Tag a test or a test module with `:logged_in` to get a client that signs # its requests. setup tags do - credentials = if tags[:logged_in], do: [consumer: @consumer, token: @token], else: [] + credentials = if tags[:logged_in], do: [credentials: @oauth], else: [] {:ok, client: Discovergy.Client.new([http_client: TestClient] ++ credentials)} end @@ -42,6 +45,10 @@ defmodule Discovergy.Case do :ok end + def authorization(headers) do + with {_name, value} <- List.keyfind(headers, "authorization", 0), do: value + end + def json(data) do {:ok, 200, [{"content-type", "application/json; charset=utf-8"}], Jason.encode!(data)} end From 5f9fb44fc3ab2cec39fd55aa39b3af505938fc83 Mon Sep 17 00:00:00 2001 From: Adrian Kumpf <8999358+adriankumpf@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:08:13 +0200 Subject: [PATCH 2/2] Drop the serialization advice from credentials/1 It prescribed :erlang.term_to_binary/1 on the grounds that the structs "redact themselves when inspected and encode to no other format", which confuses two unrelated things: Inspect governs display, and term_to_binary never consults it. It was also wrong for the common case, since a term kept in ETS, a GenServer, :persistent_term or an Agent is never serialized at all, and the warning about blobs from older versions was generic struct advice rather than anything about this library. How a caller stores an opaque term is not the library's business. What is: an OAuth session saves two rate limited calls, and a Basic auth one is not worth storing at all. --- lib/discovergy/client.ex | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/discovergy/client.ex b/lib/discovergy/client.ex index f4e85b5..64953e4 100644 --- a/lib/discovergy/client.ex +++ b/lib/discovergy/client.ex @@ -73,15 +73,12 @@ defmodule Discovergy.Client do @doc """ Returns the credentials of the session, or `nil` if the client has none. - Worth persisting for an OAuth session, which `new/1` takes back to skip the - two rate limited calls `login/3` would spend after a restart. Serialize it - with `:erlang.term_to_binary/1`, since the structs redact themselves when - inspected and encode to no other format, and be ready for a blob written by - an older version of this library not to load. - - A Basic auth session is worth nothing persisted: `basic_auth/3` rebuilds it - from the same email and password, and storing it writes the account's - password somewhere new. + Worth keeping for an OAuth session: handing it back to `new/1` after a + restart skips the two rate limited calls `login/3` would spend. + + A Basic auth session is not worth keeping. `basic_auth/3` rebuilds it from + the same email and password, and storing it puts the account's password + somewhere new. ## Examples