Skip to content

Commit 7db4be6

Browse files
committed
feat(security): Add user confirmation/reauth for sensitive changes
Auditors can raise Reauth(reason) exception to require the user to enter a token (e.g. account password) to verify the user is performing the change. Naming is subject to change. actions.py: New ReauthAction class handler and verifyPassword() method for overriding if needed. client.py: Handle Reauth exception by calling Client:reauth() method. Default client:reauth method. Add 'reauth' action declaration. exceptions.py: Define and document Reauth exception as a subclass of RoundupCGIException. templating.py: Define method utils.embed_form_fields(). The original form making a change to the database has a lot of form fields. These need to be resubmitted to Roundup as part of the form submission that verifies the user's password. This method turns all non file form fields into type=hidden inputs. It escapes the names and values to prevent XSS. For file form fields, it base64 encodes the contents and puts them in hidden pre blocks. The pre blocks have data attributes for the filename, filetype and the original field name. (Note the original field name is not used.) This stops the file content data (maybe binary e.g. jpegs) from breaking the html page. The reauth template runs JavaScript that turns the encoded data inside the pre tags back into a file. Then it adds a multiple file input control to the page and attaches all the files to it. This file input is submitted with the rest of the fields. _generic.reauth.html (multiple tracker templates): Generates a form with id=reauth_form to: display any message from the Reauth exception to the user (e.g. why user is asked to auth). get the user's password submit the form embed all the form data that triggered the reauth recreate any file data that was submitted as part of the form and generate a new file input to push the data to the back end It has the JavaScript routine (as an IIFE) that regenerates a file input without user intervention. All the TAL based tracker templates use the same form. There is also one for the jinja2 template. The JavaScript for both is the same. reference.txt: document embed_form_fields utility method. upgrading.txt: initial upgrading docs. TODO: Finalize naming. I am leaning toward ConfirmID rather than Reauth. Still looking for a standard name for this workflow. Externalize the javascript in _generic.reauth.html to a seperate file and use utils.readfile() to embed it or change the script to load it from a @@file url. Clean up upgrading.txt with just steps to implement and less feature detail/internals. Document internals/troubleshooting in reference.txt. Add tests using live server.
1 parent 72d453a commit 7db4be6

File tree

12 files changed

+995
-2
lines changed

12 files changed

+995
-2
lines changed

CHANGES.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ Fixed:
2626

2727
Features:
2828

29+
- add support for authorized changes. User can be prompted to enter
30+
their password to authorize a change. If the user's password is
31+
properly entered, the change is committed. (John Rouillard)
32+
2933
2025-07-13 2.5.0
3034

3135
Fixed:

doc/reference.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3577,6 +3577,11 @@ be added to the variable by using extensions_.
35773577
- set_http_response sets the HTTP response code for the request.
35783578
* - :meth:`url_quote <roundup.cgi.templating.TemplatingUtils.url_quote>`
35793579
- quote some text as safe for a URL (ie. space, %, ...)
3580+
* - :meth:`embed_form_fields <roundup.cgi.templating.TemplatingUtils.embed_form_fields>`
3581+
- Creates a hidden input for each of the client's form fields. It
3582+
also embeds base64 encoded file contents into pre tags and
3583+
processes that informtion back into a file input control.
3584+
35803585

35813586
-----
35823587

doc/upgrading.txt

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,71 @@ your database.
103103

104104
</details>
105105

106+
.. index:: Upgrading; 2.5.0 to 2.6.0
107+
108+
Migrating from 2.5.0 to 2.6.0
109+
=============================
110+
111+
Support authorized changes in your tracker (optional)
112+
-----------------------------------------------------
113+
114+
An auditor can require change verification with user's password.
115+
116+
When changing sensitive information (e.g. passwords) it is
117+
useful to ask for a validated authorization. This makes sure
118+
that the user is present by typing their password.
119+
120+
In an auditor adding::
121+
122+
from roundup.cgi.exceptions import Reauth
123+
124+
if 'password' in newvalues and not getattr(db, 'reauth_done', False):
125+
raise Reauth('Add an optional message to the user')
126+
127+
will present the user with a authorization page and optional message
128+
when the password is changed. The page is generated from the
129+
``_generic.reauth.html`` template by default.
130+
131+
Once the user enters their password and submits the page, the
132+
password will be verified using:
133+
134+
roundup.cgi.actions:LoginAction::verifyPassword()
135+
136+
If the password is correct the original change is done.
137+
138+
To prevent the auditor from trigering another Reauth, the
139+
attribute ``reauth_done`` is added to the db object. As a result,
140+
the getattr call will return True and not raise Reauth.
141+
142+
You get one reauth for a submitted change. Note you cannot Reauth
143+
multiple properties separately. If you need to auth multiple
144+
properties separately, you need to reject the change and force the
145+
user to submit each sensitive property separately. For example::
146+
147+
if 'password' in newvalues and 'realname' in newvalues:
148+
raise Reject('Changing the username and the realname '
149+
'at the same time is not allowed. Please '
150+
'submit two changes.')
151+
152+
if 'password' in newvalues and not getattr(db, 'reauth_done', False):
153+
raise Reauth()
154+
155+
if 'realname' in newvalues and not getattr(db, 'reauth_done', False):
156+
raise Reauth()
157+
158+
See also: client.py:Client:reauth(self, exception) which can be changed
159+
using interfaces.py in your tracker.
160+
161+
You should copy ``_generic.reauth.html`` into your tracker's html
162+
subdirectory. See the classic template directory for a copy. If you
163+
are using jinja2, see the jinja2 template directory. Then you can
164+
raise a Reauth exception and have the proper page displayed.
165+
166+
Also javascript *MUST* be turned on if this is used with a file
167+
input. If JavaScript is not turned on, attached files are lost during
168+
the reauth step. Information from other types of inputs (password,
169+
date, text etc.) do not need JavaScript to work.
170+
106171
.. index:: Upgrading; 2.4.0 to 2.5.0
107172

108173
Migrating from 2.4.0 to 2.5.0

roundup/cgi/actions.py

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
'SearchAction',
2424
'EditCSVAction', 'EditItemAction', 'PassResetAction',
2525
'ConfRegoAction', 'RegisterAction', 'LoginAction', 'LogoutAction',
26-
'NewItemAction', 'ExportCSVAction', 'ExportCSVWithIdAction']
26+
'NewItemAction', 'ExportCSVAction', 'ExportCSVWithIdAction',
27+
'ReauthAction']
2728

2829

2930
class Action:
@@ -1843,6 +1844,134 @@ def handle(self):
18431844

18441845
return '\n'
18451846

1847+
class ReauthAction(Action):
1848+
'''Allow an auditor to require change verification with user's password.
1849+
1850+
When changing sensitive information (e.g. passwords) it is
1851+
useful to ask for a validated authorization. This makes sure
1852+
that the user is present by typing their password.
1853+
1854+
In an auditor adding::
1855+
1856+
if 'password' in newvalues and not getattr(db, 'reauth_done', False):
1857+
raise Reauth()
1858+
1859+
will present the user with a authorization page when the
1860+
password is changed. The page is generated from the
1861+
_generic.reauth.html template by default.
1862+
1863+
Once the user enters their password and submits the page, the
1864+
password will be verified using:
1865+
roundup.cgi.actions:LoginAction::verifyPassword(). If the
1866+
password is correct the original change is done.
1867+
1868+
To prevent the auditor from trigering another Reauth, the
1869+
attribute "reauth_done" is added to the db object. As a result,
1870+
the getattr call will return True and not raise Reauth.
1871+
1872+
You get one reauth for the submitted change. Note you cannot
1873+
Reauth multiple properties separately. If you need to auth
1874+
multiple properties separately, you need to reject the change
1875+
and force the user to submit each sensitive property separately.
1876+
For example::
1877+
1878+
if 'password' in newvalues and 'realname' in newvalues:
1879+
raise Reject('Changing the username and the realname '
1880+
'at the same time is not allowed. Please '
1881+
'submit two changes.')
1882+
1883+
if 'password' in newvalues and not getattr(db, 'reauth_done', False):
1884+
raise Reauth()
1885+
1886+
if 'realname' in newvalues and not getattr(db, 'reauth_done', False):
1887+
raise Reauth()
1888+
1889+
Limitations: It can not handle file input fields.
1890+
1891+
See also: client.py:Client:reauth() which can be changed
1892+
using interfaces.py in your tracker.
1893+
'''
1894+
1895+
def handle(self):
1896+
''' Handle a form with a reauth request.
1897+
'''
1898+
1899+
if self.client.env['REQUEST_METHOD'] != 'POST':
1900+
raise Reject(self._('Invalid request'))
1901+
1902+
if '@reauth_password' not in self.form:
1903+
self.client.add_error_message(self._('Password incorrect.'))
1904+
return
1905+
1906+
# Verify password
1907+
login = self.client.get_action_class('login')(self.client)
1908+
if not self.verifyPassword():
1909+
self.client.add_error_message(self._("Password incorrect."))
1910+
self.client.template = "reauth"
1911+
1912+
# Strip fields that are added by the template. All
1913+
# the rest of the fields in self.form.list are added
1914+
# as hidden fields by the reauth template.
1915+
self.form.list = [ x for x in self.form.list
1916+
if x.name not in (
1917+
'@action',
1918+
'@csrf',
1919+
'@reauth_password',
1920+
'@template',
1921+
'submit'
1922+
)
1923+
]
1924+
return
1925+
1926+
# extract the info we need to preserve/reinject into
1927+
# the next action.
1928+
1929+
if '@next_action' not in self.form: # required to look up next action
1930+
self.client.add_error_message(
1931+
self._('Missing action to be authorized.'))
1932+
return
1933+
1934+
action = self.form['@next_action'].value.lower()
1935+
1936+
next_template = None; # optional
1937+
if '@next_template' in self.form:
1938+
next_template = self.form['@next_template'].value
1939+
1940+
# rewrite the form for redisplay
1941+
# remove all the reauth_form_fields leaving just the original
1942+
# form fields from the form that trigered the reauth.
1943+
# We extracted @next_* above to route to the original action
1944+
# and template.
1945+
1946+
reauth_form_fields = ('@reauth_password', '@template',
1947+
'@next_template', '@action',
1948+
'@next_action', '@csrf', 'submit')
1949+
1950+
self.form.list = [ x for x in self.form.list
1951+
if x.name not in reauth_form_fields
1952+
]
1953+
1954+
try:
1955+
action_klass = self.client.get_action_class(action)
1956+
1957+
# set the template to go back to
1958+
# use "" not None as this value gets encoded and None is invalid
1959+
self.client.template = next_template if next_template else ""
1960+
# use this in detector (to skip reauth
1961+
self.client.db.reauth_done = True
1962+
1963+
# should raise exception Redirect
1964+
action_klass(self.client).execute()
1965+
1966+
except (ValueError, Reject) as err:
1967+
escape = not isinstance(err, RejectRaw)
1968+
self.add_error_message(str(err), escape=escape)
1969+
1970+
def verifyPassword(self):
1971+
login = self.client.get_action_class('login')(self.client)
1972+
return login.verifyPassword(self.userid,
1973+
self.form['@reauth_password'].value)
1974+
18461975

18471976
class Bridge(BaseAction):
18481977
"""Make roundup.actions.Action executable via CGI request.

roundup/cgi/client.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class SysCallError(Exception):
4141
NotFound,
4242
NotModified,
4343
RateLimitExceeded,
44+
Reauth,
4445
Redirect,
4546
SendFile,
4647
SendStaticFile,
@@ -960,6 +961,8 @@ def inner_main(self):
960961

961962
except SeriousError as message:
962963
self.write_html(str(message))
964+
except Reauth as e:
965+
self.reauth(e)
963966
except Redirect as url:
964967
# let's redirect - if the url isn't None, then we need to do
965968
# the headers, otherwise the headers have been set before the
@@ -1916,6 +1919,44 @@ def determine_context(self, dre=dre_url):
19161919
if template_override is not None:
19171920
self.template = template_override
19181921

1922+
def reauth(self, exception):
1923+
"""Processing for a Reauth exception raised from an auditor.
1924+
1925+
Can be overridden by code in tracker's interfaces.py.
1926+
"""
1927+
1928+
from roundup.anypy.vendored.cgi import MiniFieldStorage
1929+
1930+
original_action = self.form['@action'].value if '@action' \
1931+
in self.form else ""
1932+
original_template = self.template
1933+
1934+
self.template = 'reauth'
1935+
self.form.list = [ x for x in self.form.list
1936+
if x.name not in ('@action',
1937+
'@csrf',
1938+
'@template'
1939+
)]
1940+
1941+
# save the action and template used when the Reauth as
1942+
# triggered. Will be used to resolve the change by the reauth
1943+
# action when when reauth password verified.
1944+
if '@next_action' not in self.form.list:
1945+
self.form.list.append(MiniFieldStorage('@next_action',
1946+
original_action))
1947+
if '@next_template' not in self.form.list:
1948+
self.form.list.append(MiniFieldStorage('@next_template',
1949+
original_template))
1950+
1951+
if exception.args and "@reauth_message" not in self.form.list:
1952+
self.form.list.append(
1953+
MiniFieldStorage('@reauth_message',
1954+
html_escape(exception.args[0])
1955+
)
1956+
)
1957+
1958+
self.write_html(self.renderContext())
1959+
19191960
# re for splitting designator, see also dre_url above this one
19201961
# doesn't strip leading 0's from the id. Why not??
19211962
dre = re.compile(r'([^\d]+)(\d+)')
@@ -2365,6 +2406,7 @@ def renderError(self, error, response_code=400, use_template=True):
23652406
('show', actions.ShowAction), # noqa: E241
23662407
('export_csv', actions.ExportCSVAction), # noqa: E241
23672408
('export_csv_id', actions.ExportCSVWithIdAction), # noqa: E241
2409+
('reauth', actions.ReauthAction), # noqa: E241
23682410
)
23692411

23702412
def handle_action(self):

roundup/cgi/exceptions.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,27 @@ class RoundupCGIException(RoundupException):
1414
pass
1515

1616

17+
class Reauth(RoundupCGIException):
18+
"""Raised by auditor, to trigger password verification before commit.
19+
20+
Before committing changes to sensitive fields, triggers the
21+
following workflow:
22+
23+
1. user is presented with a page to enter his/her password
24+
2. page is submitted to reauth action
25+
3. password is verified by LoginAction.verifyPassword
26+
4. change is committed to database.
27+
28+
If 3 fails, restart at step 1.
29+
30+
Client.reauth() method is used to handle step 1. Should be
31+
able to be overridden in interfaces.py. Step 2 is done by
32+
_generic.reauth.html. Steps 3 and 4 (and cycle back to 1) is
33+
done by cgi/actions.py:Reauth action.
34+
"""
35+
pass
36+
37+
1738
class HTTPException(RoundupCGIException):
1839
"""Base exception for all HTTP error codes."""
1940
pass

0 commit comments

Comments
 (0)