don't throw error if an access token can't be parsed as JWT

it's not technically required that an access token is a JWT

Author: mackuba

CHANGELOG.md

@@ -1,13 +1,19 @@
 ## Unreleased
 
+The "really niche bugfix" edition:
+
 * don't stop fetching in `fetch_all` if an empty page is returned but the cursor is not nil; it's technically allowed for the server to return an empty page but still have more data to send
 * in `post_request`, don't set Content-Type to "application/json" if the data sent is a string or nil (it might cause an error in some cases, like when uploading some binary content)
+* handle the (somewhat theoretical but possible) case where an access token is not a JWT but just some opaque blob – in that case, Minisky will now not throw an error trying to parse it, but just treat it as "unknown" and will not try to refresh it
+  - note: at the moment Minisky will not catch the "token expired" error and refresh the token automatically in such scenario
 * allow connecting to non-HTTPS servers (e.g. `http://localhost:3000`)
 * allow making unauthenticated clients with custom classes by returning `nil` from `#config`; custom clients with a config that's missing an `id` or `pass` are treated as an error
 * deprecate logging in using an email address in the `id` field – `createSession` accepts such identifier, but unlike with handle or DID, there's no way to use it to look up the DID document and PDS location if we wanted to
 * fixed URL query params in POST requests on Ruby 2.x
 * marked `Minisky#active_repl?` method as private
 
+Also added YARD API documentation for most of the code.
+
 ## [0.5.0] - 2024-12-27 🎄
 
 * `host` param in the initializer can be passed with a `https://` prefix (useful if you're passing it directly from a DID document, e.g. using DIDKit)

lib/minisky/requests.rb

@@ -304,12 +304,13 @@ def fetch_all(method, params = nil, auth: default_auth_mode,
     #   - `:logged_in` if a login using a password was performed
     #   - `:refreshed` if the access token was expired and was refreshed
     #   - `:ok` if no refresh was needed
+    #   - `:unknown` if the token is not a valid JWT (e.g. an opaque blob)
     #
     # @raise [BadResponse] if login or refresh returns an error status code
     # @raise [AuthError]
+    #   - if the client doesn't include user config at all
     #   - if logging in is required, but login or password isn't provided
     #   - if token refresh is needed, but refresh token is missing
-    #   - if a token has invalid format
 
     def check_access
       if !user
@@ -318,8 +319,16 @@ def check_access
         raise AuthError, "User id or password is missing"
       elsif !user.logged_in?
         log_in
-        :logged_in
-      elsif access_token_expired?
+        return :logged_in
+      end
+
+      begin
+        expired = access_token_expired?
+      rescue AuthError
+        return :unknown
+      end
+
+      if expired
         perform_token_refresh
         :refreshed
       else
@@ -390,29 +399,58 @@ def perform_token_refresh
       json
     end
 
+    # Attempts to parse a given token as JWT and extract the expiration date from the payload.
+    # An access token technically isn't required to be a (valid) JWT, so if the parsing fails
+    # for whatever reason, nil is returned.
+    #
+    # @return [Time, nil] parsed expiration time, or nil if token is not a valid JWT
+
     def token_expiration_date(token)
+      return nil unless token.valid_encoding?
+
       parts = token.split('.')
-      raise AuthError, "Invalid access token format" unless parts.length == 3
+      return nil unless parts.length == 3
 
       begin
         payload = JSON.parse(Base64.decode64(parts[1]))
       rescue JSON::ParserError
-        raise AuthError, "Couldn't decode payload from access token"
+        return nil
       end
 
       exp = payload['exp']
-      raise AuthError, "Invalid token expiry data" unless exp.is_a?(Numeric) && exp > 0
+      return nil unless exp.is_a?(Numeric) && exp > 0
 
-      Time.at(exp)
+      time = Time.at(exp)
+      return nil if time.year < 2023 || time.year > 2100
+
+      time
     end
 
+    # Attempts to parse the user's access token as JWT, extract the expiration date from the
+    # payload, and check if the token hasn't expired yet.
+    #
+    # @return [Boolean] true if the token's expiration time is more than a minute away
+    # @raise [AuthError] if the token is not a valid JWT, or user is not logged in
+
     def access_token_expired?
-      token_expiration_date(user.access_token) < Time.now + 60
+      if user&.access_token.nil?
+        raise AuthError, "No access token (user is not logged in)"
+      end
+
+      exp_date = token_expiration_date(user.access_token)
+
+      if exp_date
+        exp_date < Time.now + 60
+      else
+        raise AuthError, "Token expiration date cannot be decoded"
+      end
     end
 
     #
     # Clear stored access and refresh tokens, effectively logging out the user.
     #
+    # @raise [AuthError] if the client doesn't have a user config
+    #
 
     def reset_tokens
       if !user

spec/minisky_spec.rb

@@ -147,6 +147,116 @@
     minisky.auto_manage_tokens.should == false
     minisky.default_progress.should == '*'
   end
+
+  describe '#token_expiration_date' do
+    subject { Minisky.new(host, nil) }
+
+    it 'should return nil for tokens with invalid encoding' do
+      token = "bad\xC3".force_encoding('UTF-8')
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return nil when the token does not have three parts' do
+      subject.token_expiration_date('token').should be_nil
+      subject.token_expiration_date('one.two').should be_nil
+      subject.token_expiration_date('1.2.3.4').should be_nil
+
+      token = make_token(Time.now + 3600)
+      subject.token_expiration_date(token + '.qwe').should be_nil
+    end
+
+    it 'should return nil when the payload is not valid JSON' do
+      token = ['header', Base64.strict_encode64('nope'), 'sig'].join('.')
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return nil when exp field is missing' do
+      token = ['header', Base64.strict_encode64(JSON.generate({ 'aud' => 'aaaa' })), 'sig'].join('.')
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return nil when exp field is not a number' do
+      token = ['header', Base64.strict_encode64(JSON.generate({ 'exp' => 'soon' })), 'sig'].join('.')
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return nil when exp field is not a positive number' do
+      token = ['header', Base64.strict_encode64(JSON.generate({ 'exp' => 0 })), 'sig'].join('.')
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return nil when expiration year is before 2023' do
+      token = make_token(Time.utc(2022, 12, 24, 19, 00, 00))
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return nil when expiration year is after 2100' do
+      token = make_token(Time.utc(2101, 5, 5, 0, 0, 0))
+      subject.token_expiration_date(token).should be_nil
+    end
+
+    it 'should return the expiration time for a valid token' do
+      time = Time.at(Time.now.to_i + 7200)
+      token = make_token(time)
+      subject.token_expiration_date(token).should == time
+    end
+  end
+
+  describe '#access_token_expired?' do
+    let(:config) { data }
+
+    before do
+      File.write('myconfig.yml', YAML.dump(config))
+    end
+
+    context 'when there is no user config' do
+      subject { Minisky.new(host, nil) }
+
+      it 'should raise AuthError' do
+        expect { subject.access_token_expired? }.to raise_error(Minisky::AuthError)
+      end
+    end
+
+    context 'when access token is missing' do
+      let(:config) { data.merge('access_token' => nil) }
+
+      it 'should raise AuthError' do
+        expect { subject.access_token_expired? }.to raise_error(Minisky::AuthError)
+      end
+    end
+
+    context 'when token expiration cannot be decoded' do
+      let(:config) { data.merge('access_token' => 'blob') }
+
+      it 'should raise AuthError' do
+        expect { subject.access_token_expired? }.to raise_error(Minisky::AuthError)
+      end
+    end
+
+    context 'when token expiration date is in the past' do
+      let(:config) { data.merge('access_token' => make_token(Time.now - 30)) }
+
+      it 'should return true' do
+        subject.access_token_expired?.should == true
+      end
+    end
+
+    context 'when token expiration date is in less than 60 seconds' do
+      let(:config) { data.merge('access_token' => make_token(Time.now + 50)) }
+
+      it 'should return true' do
+        subject.access_token_expired?.should == true
+      end
+    end
+
+    context 'when token expiration date is in more than 60 seconds' do
+      let(:config) { data.merge('access_token' => make_token(Time.now + 180)) }
+
+      it 'should return false' do
+        subject.access_token_expired?.should == false
+      end
+    end
+  end
 end
 
 describe 'in Minisky instance' do

spec/spec_helper.rb

@@ -1,4 +1,6 @@
 require_relative 'spec_config'
+require 'base64'
+require 'json'
 
 INVALID_METHOD_NAMES = [
   'getUsers',
@@ -36,3 +38,11 @@ def verify_fetch_all
     WebMock.should have_requested(:get, url).once
   end
 end
+
+def make_token(exp_time)
+  header = { alg: 'HS256', typ: 'JWT' }
+  payload = { exp: exp_time.to_i }
+  signature = 'signature'
+
+  [header, payload, signature].map { |part| Base64.strict_encode64(JSON.generate(part)) }.join('.')
+end