Remove deprecated support for instantiating OAuth flows w/o callback

Author: thekid

ChangeLog.md

@@ -5,6 +5,9 @@ Web Authentication change log
 
 ## 7.0.0 / ????-??-??
 
+* **Heads up:** Removed support for deprecated constructor signature without
+  callback in OAuth1 & OAuth2
+  (@thekid)
 * Changed `SessionBased` authentication to send 401 for sub-requests (e.g.
   images, fetch(), ...), implementing feature suggested in #38
   (@thekid)

src/main/php/web/auth/oauth/OAuth1Flow.class.php

@@ -16,7 +16,7 @@ class OAuth1Flow extends OAuthFlow {
    * @param  web.auth.oauth.Credentials|(string|util.Secret)[] $consumer
    * @param  string|util.URI $callback
    */
-  public function __construct($service, $consumer, $callback= null) {
+  public function __construct($service, $consumer, $callback= '/') {
     $this->namespace= 'oauth1::flow';
     $this->service= rtrim($service, '/');
 
@@ -29,13 +29,7 @@ public function __construct($service, $consumer, $callback= null) {
       $this->signature= new Signature(new BySecret(...$consumer));
     }
 
-    // BC: Support deprecated constructor signature without callback
-    if (null === $callback) {
-      trigger_error('Missing parameter $callback', E_USER_DEPRECATED);
-      $this->callback= null;
-    } else {
-      $this->callback= $callback instanceof URI ? $callback : new URI($callback);
-    }
+    $this->callback= $callback instanceof URI ? $callback : new URI($callback);
   }
 
   /**
@@ -90,9 +84,9 @@ public function authenticate($request, $response, $session) {
       )));
     }
 
-    // Enter authentication flow, resolving callback URI against the curren request.
+    // Enter authentication flow, resolving callback URI against the current request.
     $uri= $this->url(true)->resolve($request);
-    $callback= $this->callback ? $uri->resolve($this->callback) : $this->service($uri);
+    $callback= $uri->resolve($this->callback);
 
     // Check whether we are continuing an existing authentication flow based on the
     // state given by the server and our session; or if we need to start a new one.

src/main/php/web/auth/oauth/OAuth2Flow.class.php

@@ -19,24 +19,15 @@ class OAuth2Flow extends OAuthFlow {
    * @param  string|util.URI $callback
    * @param  string[] $scopes
    */
-  public function __construct($auth, $tokens, $consumer, $callback= null, $scopes= ['user']) {
+  public function __construct($auth, $tokens, $consumer, $callback= '/', $scopes= ['user']) {
     $this->namespace= 'oauth2::flow';
     $this->auth= $auth instanceof URI ? $auth : new URI($auth);
     $this->backend= $tokens instanceof OAuth2Endpoint
       ? $tokens->using($consumer)
       : new OAuth2Endpoint($tokens, $consumer)
     ;
-
-    // BC: Support deprecated constructor signature without callback
-    if (is_array($callback) || null === $callback) {
-      trigger_error('Missing parameter $callback', E_USER_DEPRECATED);
-      $this->callback= null;
-      $this->scopes= $callback ?? $scopes;
-    } else {
-      $this->callback= $callback instanceof URI ? $callback : new URI($callback);
-      $this->scopes= $scopes;
-    }
-
+    $this->callback= $callback instanceof URI ? $callback : new URI($callback);
+    $this->scopes= $scopes;
     $this->rand= new Random();
   }
 
@@ -87,9 +78,9 @@ public function authenticate($request, $response, $session) {
       return ByAccessToken::from($token);
     }
 
-    // Enter authentication flow, resolving callback URI against the curren request.
+    // Enter authentication flow, resolving callback URI against the current request.
     $uri= $this->url(true)->resolve($request);
-    $callback= $this->callback ? $uri->resolve($this->callback) : $this->service($uri);
+    $callback= $uri->resolve($this->callback);
 
     // Check whether we are continuing an existing authentication flow based on the
     // state given by the server and our session; or if we need to start a new one.

src/main/php/web/auth/oauth/OAuthFlow.class.php

@@ -15,12 +15,12 @@ protected function flow($state, $stored) {
     );
   }
 
-  /** @return ?util.URI */
+  /** @return util.URI */
   public function callback() { return $this->callback; }
 
-  /** @param ?string|util.URI $callback */
+  /** @param string|util.URI $callback */
   public function calling($callback): self {
-    $this->callback= null === $callback || $callback instanceof URI ? $callback : new URI($callback);
+    $this->callback= $callback instanceof URI ? $callback : new URI($callback);
     return $this;
   }
 

src/test/php/web/auth/unittest/OAuth1FlowTest.class.php

@@ -164,22 +164,6 @@ public function replaces_fragment($fragment) {
     Assert::equals(['uri' => 'http://localhost/#'.$fragment, 'seed' => []], current($session->value(self::SNS)['flows']));
   }
 
-  /** @deprecated */
-  #[Test, Values(from: 'paths')]
-  public function deprecated_usage_without_callback_uri($path) {
-    $request= ['oauth_token' => 'T'];
-    $fixture= newinstance(OAuth1Flow::class, [self::AUTH, [self::ID, self::SECRET]], [
-      'request' => function($path, $token= null, $params= []) use($request) { return $request; }
-    ]);
-    \xp::gc();
-    $session= (new ForTesting())->create();
-
-    Assert::equals(
-      sprintf('%s/authenticate?oauth_token=T&oauth_callback=%s', self::AUTH, urlencode('http://localhost'.$path)),
-      $this->redirectTo($this->authenticate($fixture, $path, $session))
-    );
-  }
-
   #[Test]
   public function use_returned_client() {
     $flow= new OAuth1Flow(self::AUTH, [self::ID, self::SECRET], self::CALLBACK);

src/test/php/web/auth/unittest/OAuth2FlowTest.class.php

@@ -375,36 +375,6 @@ public function claims_returned_with_expiry() {
     );
   }
 
-  /** @deprecated */
-  #[Test, Values(from: 'paths')]
-  public function deprecated_usage_without_callback_uri($path) {
-    $fixture= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER);
-    \xp::gc();
-
-    $session= (new ForTesting())->create();
-    $this->assertLoginWith(
-      'http://localhost'.$path,
-      $fixture->scopes(),
-      $this->authenticate($fixture, $path, $session),
-      $session
-    );
-  }
-
-  /** @deprecated */
-  #[Test, Values(from: 'paths')]
-  public function deprecated_usage_with_scopes_in_place_of_callback_uri($path) {
-    $fixture= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER, ['user']);
-    \xp::gc();
-
-    $session= (new ForTesting())->create();
-    $this->assertLoginWith(
-      'http://localhost'.$path,
-      $fixture->scopes(),
-      $this->authenticate($fixture, $path, $session),
-      $session
-    );
-  }
-
   #[Test]
   public function use_returned_client() {
     $flow= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER, self::CALLBACK);