Zfilipin has submitted this change and it was merged.

Change subject: Update account creation code for AuthManager
......................................................................


Update account creation code for AuthManager

AuthManager (with $wgAuthManagerDisabled=false)changes the account
creation API, which many browser tests relied on; that resulted in
mass test breakage after I6695aa3da42fb2b088eaa8d1883ccbb67f2c0c38.

This change updates the gem to test which API flavor is in use,
then send requests accordingly.

Bug: T135884
Change-Id: Iaf1db8846c8c79a4b8b8bc68a749d5b1ce054a52
---
M lib/mediawiki_api/client.rb
M spec/client_spec.rb
2 files changed, 141 insertions(+), 35 deletions(-)

Approvals:
  Zfilipin: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/lib/mediawiki_api/client.rb b/lib/mediawiki_api/client.rb
index 5a4d80d..e66996f 100644
--- a/lib/mediawiki_api/client.rb
+++ b/lib/mediawiki_api/client.rb
@@ -41,7 +41,44 @@
       end
     end
 
-    def create_account(username, password, token = nil)
+    def create_account(username, password)
+      params = { modules: 'createaccount', token_type: false }
+      d = action(:paraminfo, params).data
+      params = d['modules'] && d['modules'][0] && d['modules'][0]['parameters']
+      if !params || !params.map
+        raise CreateAccountError, 'unexpected API response format'
+      end
+      params = params.map{ |o| o['name'] }
+
+      if params.include? 'requests'
+        create_account_new(username, password)
+      else
+        create_account_old(username, password)
+      end
+    end
+
+    def create_account_new(username, password)
+      # post-AuthManager
+      data = action(:query, { meta: 'tokens', type: 'createaccount', 
token_type: false }).data
+      token = data['tokens'] && data['tokens']['createaccounttoken']
+      unless token
+        raise CreateAccountError, 'failed to get createaccount API token'
+      end
+
+      data = action(:createaccount, {
+        username: username,
+        password: password,
+        retype: password,
+        createreturnurl: 'http://example.com', # won't be used but must be a 
valid URL
+        createtoken: token,
+        token_type: false
+      }).data
+      raise CreateAccountError, data['message'] if data['status'] != 'PASS'
+      data
+    end
+
+    def create_account_old(username, password, token = nil)
+      # pre-AuthManager
       params = { name: username, password: password, token_type: false }
       params[:token] = token unless token.nil?
 
@@ -52,7 +89,7 @@
         @logged_in = true
         @tokens.clear
       when 'NeedToken'
-        data = create_account(username, password, data['token'])
+        data = create_account_old(username, password, data['token'])
       else
         raise CreateAccountError, data['result']
       end
diff --git a/spec/client_spec.rb b/spec/client_spec.rb
index 64aaf21..3681820 100644
--- a/spec/client_spec.rb
+++ b/spec/client_spec.rb
@@ -335,57 +335,126 @@
   end
 
   describe '#create_account' do
-    it 'creates an account when API returns Success' do
-      stub_request(:post, api_url).
-        with(body: { format: 'json', action: 'createaccount', name: 'Test', 
password: 'qwe123' }).
-        to_return(body: { createaccount: body_base.merge(result: 'Success') 
}.to_json)
-
-      expect(subject.create_account('Test', 'qwe123')).to include('result' => 
'Success')
-    end
-
-    context 'when API returns NeedToken' do
+    context 'when the old createaccount API is used' do
       before do
         stub_request(:post, api_url).
-          with(body: { format: 'json', action: 'createaccount',
-                       name: 'Test', password: 'qwe123' }).
-          to_return(
-            body: { createaccount: body_base.merge(result: 'NeedToken', token: 
'456') }.to_json,
-            headers: { 'Set-Cookie' => 'prefixSession=789; path=/; 
domain=localhost; HttpOnly' }
-          )
-
-        @success_req = stub_request(:post, api_url).
-          with(body: { format: 'json', action: 'createaccount',
-                       name: 'Test', password: 'qwe123', token: '456' }).
-          with(headers: { 'Cookie' => 'prefixSession=789' }).
-          to_return(body: { createaccount: body_base.merge(result: 'Success') 
}.to_json)
+          with(body: { action: 'paraminfo', format: 'json', modules: 
'createaccount' }).
+          to_return(body: { paraminfo: body_base.merge(modules: [{ parameters: 
[] }]) }.to_json)
       end
 
-      it 'creates an account' do
+      it 'creates an account when API returns Success' do
+        stub_request(:post, api_url).
+          with(body: { format: 'json', action: 'createaccount', name: 'Test', 
password: 'qwe123' }).
+          to_return(body: { createaccount: body_base.merge(result: 'Success') 
}.to_json)
+
         expect(subject.create_account('Test', 'qwe123')).to include('result' 
=> 'Success')
       end
 
-      it 'sends second request with token and cookies' do
-        subject.create_account 'Test', 'qwe123'
-        expect(@success_req).to have_been_requested
+      context 'when API returns NeedToken' do
+        before do
+          stub_request(:post, api_url).
+            with(body: { format: 'json', action: 'createaccount',
+                         name: 'Test', password: 'qwe123' }).
+            to_return(
+              body: { createaccount: body_base.merge(result: 'NeedToken', 
token: '456') }.to_json,
+              headers: { 'Set-Cookie' => 'prefixSession=789; path=/; 
domain=localhost; HttpOnly' }
+            )
+
+          @success_req = stub_request(:post, api_url).
+            with(body: { format: 'json', action: 'createaccount',
+                         name: 'Test', password: 'qwe123', token: '456' }).
+            with(headers: { 'Cookie' => 'prefixSession=789' }).
+            to_return(body: { createaccount: body_base.merge(result: 
'Success') }.to_json)
+        end
+
+        it 'creates an account' do
+          expect(subject.create_account('Test', 'qwe123')).to include('result' 
=> 'Success')
+        end
+
+        it 'sends second request with token and cookies' do
+          subject.create_account 'Test', 'qwe123'
+          expect(@success_req).to have_been_requested
+        end
+      end
+
+      # docs don't specify other results, but who knows
+      # http://www.mediawiki.org/wiki/API:Account_creation
+      context 'when API returns neither Success nor NeedToken' do
+        before do
+          stub_request(:post, api_url).
+            with(body: { format: 'json', action: 'createaccount',
+                         name: 'Test', password: 'qwe123' }).
+            to_return(body: { createaccount: body_base.merge(result: 
'WhoKnows') }.to_json)
+        end
+
+        it 'raises error with proper message' do
+          expect { subject.create_account 'Test', 'qwe123' }.to raise_error(
+            MediawikiApi::CreateAccountError,
+            'WhoKnows'
+          )
+        end
       end
     end
 
-    # docs don't specify other results, but who knows
-    # http://www.mediawiki.org/wiki/API:Account_creation
-    context 'when API returns neither Success nor NeedToken' do
+    context 'when the new createaccount API is used' do
       before do
         stub_request(:post, api_url).
-          with(body: { format: 'json', action: 'createaccount',
-                       name: 'Test', password: 'qwe123' }).
-          to_return(body: { createaccount: body_base.merge(result: 'WhoKnows') 
}.to_json)
+          with(body: { action: 'paraminfo', format: 'json', modules: 
'createaccount' }).
+                  to_return(body: { paraminfo: body_base.merge(
+                    modules: [{ parameters: [{ name: 'requests' }] }]
+                  ) }.to_json)
       end
 
-      it 'raises error with proper message' do
+      it 'raises an error when fetching a token fails' do
+        stub_request(:post, api_url).
+          with(body: { action: 'query', format: 'json', meta: 'tokens', type: 
'createaccount' }).
+          to_return(body: { tokens: body_base.merge(foo: '12345\\+')  
}.to_json)
         expect { subject.create_account 'Test', 'qwe123' }.to raise_error(
           MediawikiApi::CreateAccountError,
-          'WhoKnows'
+          'failed to get createaccount API token'
         )
       end
+
+      context 'when fetching a token succeeds' do
+        before do
+          stub_request(:post, api_url).
+            with(body: { format: 'json', action: 'query', meta: 'tokens', 
type: 'createaccount' }).
+            to_return(body: { tokens: body_base.merge(createaccounttoken: 
'12345\\+') }.to_json)
+        end
+
+        it 'creates an account when the API returns success' do
+          stub_request(:post, api_url).
+            with(body: { format: 'json', action: 'createaccount',
+                         createreturnurl: 'http://example.com', username: 
'Test',
+                         password: 'qwe123', retype: 'qwe123', createtoken: 
'12345\\+' }).
+            to_return(body: { createaccount: body_base.merge(status: 'PASS') 
}.to_json)
+          expect(subject.create_account('Test', 'qwe123')).to include('status' 
=> 'PASS')
+        end
+
+        it 'raises an error when the API returns failure' do
+          stub_request(:post, api_url).
+            with(body: { format: 'json', action: 'createaccount',
+                         createreturnurl: 'http://example.com', username: 
'Test',
+                         password: 'qwe123', retype: 'qwe123', createtoken: 
'12345\\+' }).
+            to_return(body: { createaccount: body_base.merge(
+              status: 'FAIL', message: 'User exists!'
+            ) }.to_json)
+          expect { subject.create_account 'Test', 'qwe123' }.to raise_error(
+            MediawikiApi::CreateAccountError,
+            'User exists!'
+          )
+        end
+      end
+    end
+
+    it 'raises an error when the paraminfo query result is weird' do
+      stub_request(:post, api_url).
+        with(body: { action: 'paraminfo', format: 'json', modules: 
'createaccount' }).
+        to_return(body: { paraminfo: body_base.merge(modules: []) }.to_json)
+      expect { subject.create_account 'Test', 'qwe123' }.to raise_error(
+        MediawikiApi::CreateAccountError,
+        'unexpected API response format'
+      )
     end
   end
 

-- 
To view, visit https://gerrit.wikimedia.org/r/290269
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iaf1db8846c8c79a4b8b8bc68a749d5b1ce054a52
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/ruby/api
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Chad <ch...@wikimedia.org>
Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Zfilipin <zfili...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to