Skip to content

Commit

Permalink
Merge pull request #3145 from tardyp/authz_smokes
Browse files Browse the repository at this point in the history
Fix login issues on py3 and recent chrome
  • Loading branch information
tardyp committed May 2, 2017
2 parents 3456ee6 + dc01099 commit d697dcb
Show file tree
Hide file tree
Showing 25 changed files with 276 additions and 85 deletions.
9 changes: 9 additions & 0 deletions master/buildbot/monkeypatches/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from twisted.python import util
from builtins import int
from future.utils import PY3


def onlyOnce(fn):
Expand All @@ -43,6 +44,13 @@ def patch_python14653():
python14653.patch()


@onlyOnce
def patch_twisted9127():
if PY3:
from buildbot.monkeypatches import twisted9127
twisted9127.patch()


@onlyOnce
def patch_testcase_timeout():
import unittest
Expand Down Expand Up @@ -121,3 +129,4 @@ def patch_all(for_tests=False):
patch_unittest_testcase()

patch_python14653()
patch_twisted9127()
53 changes: 53 additions & 0 deletions master/buildbot/monkeypatches/twisted9127.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# This file is part of Buildbot. Buildbot is free software: you can
# redistribute it and/or modify it under the terms of the GNU General Public
# License as published by the Free Software Foundation, version 2.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
# details.
#
# You should have received a copy of the GNU General Public License along with
# this program; if not, write to the Free Software Foundation, Inc., 51
# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
#
# Copyright Buildbot Team Members

from __future__ import absolute_import
from __future__ import print_function

from twisted.python.compat import nativeString
from twisted.python.compat import networkString


# patch for http://twistedmatrix.com/trac/ticket/9127
# unfortunately the impacted code is deeply inside render method, so we need to patch the whole
# render method

def render(self, request):
"""
Send www-authenticate headers to the client
"""
def generateWWWAuthenticate(scheme, challenge):
_l = []
for k, v in challenge.items():
_l.append(networkString("%s=%s" % (nativeString(k), quoteString(nativeString(v)))))
return b" ".join([scheme, b", ".join(_l)])

def quoteString(s):
return '"%s"' % (s.replace('\\', '\\\\').replace('"', '\\"'),)

request.setResponseCode(401)
for fact in self._credentialFactories:
challenge = fact.getChallenge(request)
request.responseHeaders.addRawHeader(
b'www-authenticate',
generateWWWAuthenticate(fact.scheme, challenge))
if request.method == b'HEAD':
return b''
return b'Unauthorized'


def patch():
from twisted.web._auth.wrapper import UnauthorizedResource
UnauthorizedResource.render = render
2 changes: 2 additions & 0 deletions master/buildbot/newsfragments/auth_py3.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix :py:class:`UserPasswordAuth` authentication on ``py3`` and recent browsers. (:issue:`3162`, :issue:`3163`).
The ``py3`` fix also requires Twisted https://github.com/twisted/twisted/pull/773.
4 changes: 2 additions & 2 deletions master/buildbot/test/unit/test_www_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def updateUserInfo(request):
self.auth.updateUserInfo = mock.Mock(side_effect=updateUserInfo)

res = yield self.render_resource(self.rsrc, b'/auth/login')
self.assertEqual(res, b'')
self.assertEqual(res, {'redirected': 'h:/a/b/#/'})
self.assertFalse(self.auth.maybeAutoLogin.called)
self.auth.updateUserInfo.assert_called_with(mock.ANY)
self.assertEqual(self.master.session.user_info,
Expand All @@ -214,5 +214,5 @@ def setUp(self):
def test_render(self):
self.master.session.expire = mock.Mock()
res = yield self.render_resource(self.rsrc, b'/auth/logout')
self.assertEqual(res, b'')
self.assertEqual(res, {'redirected': 'h:/a/b/#/'})
self.master.session.expire.assert_called_with()
18 changes: 13 additions & 5 deletions master/buildbot/www/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from twisted.web.resource import IResource
from zope.interface import implementer

from buildbot.util import bytes2NativeString
from buildbot.util import config
from buildbot.www import resource

Expand Down Expand Up @@ -158,8 +159,8 @@ class HTPasswdAuth(TwistedICredAuthBase):
def __init__(self, passwdFile, **kwargs):
TwistedICredAuthBase.__init__(
self,
[DigestCredentialFactory("md5", "buildbot"),
BasicCredentialFactory("buildbot")],
[DigestCredentialFactory(b"md5", b"buildbot"),
BasicCredentialFactory(b"buildbot")],
[FilePasswordDB(passwdFile)],
**kwargs)

Expand All @@ -169,12 +170,17 @@ class UserPasswordAuth(TwistedICredAuthBase):
def __init__(self, users, **kwargs):
TwistedICredAuthBase.__init__(
self,
[DigestCredentialFactory("md5", "buildbot"),
BasicCredentialFactory("buildbot")],
[DigestCredentialFactory(b"md5", b"buildbot"),
BasicCredentialFactory(b"buildbot")],
[InMemoryUsernamePasswordDatabaseDontUse(**dict(users))],
**kwargs)


def _redirect(master, request):
url = request.args.get("redirect", ["/"])[0]
return resource.Redirect(master.config.buildbotURL + "#" + url)


class PreAuthenticatedLoginResource(LoginResource):
# a LoginResource which is already authenticated via a
# HTTPAuthSessionWrapper
Expand All @@ -186,8 +192,9 @@ def __init__(self, master, username):
@defer.inlineCallbacks
def renderLogin(self, request):
session = request.getSession()
session.user_info = dict(username=self.username)
session.user_info = dict(username=bytes2NativeString(self.username))
yield self.master.www.auth.updateUserInfo(request)
raise _redirect(self.master, request)


class LogoutResource(resource.Resource):
Expand All @@ -196,4 +203,5 @@ def render_GET(self, request):
session = request.getSession()
session.expire()
session.updateSession(request)
request.redirect(_redirect(self.master, request).url)
return b''
48 changes: 48 additions & 0 deletions master/docs/relnotes/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,54 @@ Release Notes

.. towncrier release notes start
Buildbot ``0.9.6-35-g615cb76`` ( ``2017-04-30`` )
=====================================================

Bug fixes
---------

- Fix :py:class:`UserPasswordAuth` authentication on ``py3`` and recent
browsers. (:issue:`3162`, :issue:`3163`). The ``py3`` fix also requires
Twisted https://github.com/twisted/twisted/pull/773.
- :ref:`ConsoleView` now display changes the same way as in Recent Changes
page.
- Fix issue with :ref:`ConsoleView` when no change source is configured but
still builds have ``got_revision`` property

Features
--------

- Builds ``state_string`` is now automatically computed according to the
:py:meth:`BuildStep.getResultSummary`, :py:attr:`BuildStep.description` and
``updateBuildSummaryPolicy`` from :ref:`Buildstep-Common-Parameters`. This
allows the dashboards and reporters to get a descent summary text of the
build without fetching the steps.


Buildbot ``0.9.6-35-g615cb76`` ( ``2017-04-30`` )
=====================================================

Bug fixes
---------

- Fix :py:class:`UserPasswordAuth` authentication on ``py3`` and recent
browsers. (:issue:`3162`, :issue:`3163`). The ``py3`` fix also requires
Twisted https://github.com/twisted/twisted/pull/773.
- :ref:`ConsoleView` now display changes the same way as in Recent Changes
page.
- Fix issue with :ref:`ConsoleView` when no change source is configured but
still builds have ``got_revision`` property

Features
--------

- Builds ``state_string`` is now automatically computed according to the
:py:meth:`BuildStep.getResultSummary`, :py:attr:`BuildStep.description` and
``updateBuildSummaryPolicy`` from :ref:`Buildstep-Common-Parameters`. This
allows the dashboards and reporters to get a descent summary text of the
build without fetching the steps.


Buildbot ``0.9.6`` ( ``2017-04-19`` )
=====================================================

Expand Down
4 changes: 2 additions & 2 deletions smokes/e2e/buildsnavigation.scenarios.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ describe 'previousnextlink', () ->
builder.waitNextBuildFinished(+lastbuild + 1)
builder.goBuild(+lastbuild + 2)
lastBuildURL = browser.getCurrentUrl()
builder.getPreviousButton().click()
builder.clickWhenClickable(builder.getPreviousButton())
expect(browser.getCurrentUrl()).not.toMatch(lastBuildURL)
builder.getNextButton().click()
builder.clickWhenClickable(builder.getNextButton())
expect(browser.getCurrentUrl()).toMatch(lastBuildURL)

describe 'forceandstop', () ->
Expand Down
10 changes: 4 additions & 6 deletions smokes/e2e/home.scenarios.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,23 @@
# test goal: checks the the number of element present in home page
# to test this part: two different builds need to be started


forcePage = require('./pages/force.coffee')
builderPage = require('./pages/builder.coffee')
homePage = require('./pages/home.coffee')


describe 'home page', () ->
force = null
builder = null
home = null

beforeEach(() ->
beforeEach () ->
builder = new builderPage('runtests', 'force')
force = new forcePage()
home = new homePage()
builder.goDefault()
)
home.loginUser("my@email.com", "mypass")

afterEach () ->
new homePage().waitAllBuildsFinished()
home.logOut()

it 'should go to the home page and check the different builder', () ->
builderName = {
Expand Down
2 changes: 1 addition & 1 deletion smokes/e2e/hook.scenarios.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe 'change hook', () ->
afterEach () ->
new homePage().waitAllBuildsFinished()

xit 'should create a build', () ->
it 'should create a build', () ->
builder.go()
builder.getLastSuccessBuildNumber().then (lastbuild) ->
browser.executeAsyncScript (done)->
Expand Down
6 changes: 4 additions & 2 deletions smokes/e2e/pages/about.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
# inspired by this methodology
# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/

class aboutPage
BasePage = require("./base.coffee")

class AboutPage extends BasePage
constructor: (builder) ->
@builder = builder

Expand All @@ -26,4 +28,4 @@ class aboutPage
dependenciesTitle = element.all(By.css('h2')).get(2)
expect(dependenciesTitle.getText()).toContain('Javascript dependencies')

module.exports = aboutPage
module.exports = AboutPage
33 changes: 33 additions & 0 deletions smokes/e2e/pages/base.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# this file will contains the different generic functions which
# will be called by the different tests
# inspired by this methodology
# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/


class BasePage
# accessors for elements that all pages have have (menu, login, etc)
constructor: () ->

clickWhenClickable: (element) ->
browser.wait ->
element.click().then (->
true
), ->
element.getLocation().then (l)->
element.getSize().then (s)->
console.log 'not clickable', s, l
false

logOut: ->
element.all(By.css('.avatar img')).click()
element.all(By.linkText('Logout')).click()
anonymousButton = element(By.css('.dropdown'))
expect(anonymousButton.getText()).toContain("Anonymous")

loginUser: (user, password) ->
browser.get("http://#{user}:#{password}@localhost:8010/auth/login")
anonymousButton = element(By.css('.dropdown'))
expect(anonymousButton.getText()).not.toContain("Anonymous")


module.exports = BasePage
6 changes: 4 additions & 2 deletions smokes/e2e/pages/builder.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
# inspired by this methodology
# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/

class builderPage
BasePage = require("./base.coffee")

class BuilderPage extends BasePage
constructor: (@builder, forcename) ->
@forceName=forcename

Expand Down Expand Up @@ -66,4 +68,4 @@ class builderPage
builderLink = element.all(By.linkText(@builder))
expect(builderLink.count()).toBeGreaterThan(0)

module.exports = builderPage
module.exports = BuilderPage
6 changes: 4 additions & 2 deletions smokes/e2e/pages/console.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
# inspired by this methodology
# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/

class consolePage
BasePage = require("./base.coffee")

class ConsolePage extends BasePage
constructor: ->

go: ->
browser.get('#/console')
countSuccess: () ->
element.all(By.css('.badge-status.results_SUCCESS')).count()

module.exports = consolePage
module.exports = ConsolePage
6 changes: 4 additions & 2 deletions smokes/e2e/pages/dashboard.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
# inspired by this methodology
# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/

class dashboardPage
BasePage = require("./base.coffee")

class DashboardPage extends BasePage
constructor: ->

go: ->
browser.get('#/mydashboard')

module.exports = dashboardPage
module.exports = DashboardPage
5 changes: 3 additions & 2 deletions smokes/e2e/pages/force.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
# will be called by the different tests
# inspired by this methodology
# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/
BasePage = require("./base.coffee")

class forcePage
class ForcePage extends BasePage
constructor: ->

setInputText: (cssLabel, value) ->
Expand Down Expand Up @@ -42,4 +43,4 @@ class forcePage
getStopButton: ->
return element(By.buttonText('Stop'))

module.exports = forcePage
module.exports = ForcePage

0 comments on commit d697dcb

Please sign in to comment.