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