Opened 14 years ago

Closed 13 years ago

#4135 closed enhancement (fixed)

[with patch, positive review] notebook -- user management features

Reported by: Timothy Clemans Owned by: Timothy Clemans
Priority: major Milestone: sage-4.1.2
Component: notebook Keywords:
Cc: Merged in: Sage 4.1.2.alpha0
Authors: Timothy Clemans Reviewers: Tim Dumol, Dan Drake
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

New features:

  • Reset a user's password
  • Add a new user
  • Suspend/unsuspend a user
  • Temporary password shown for each user if one is set (Created by password reset and add user)

Attachments (5)

trac_4135.patch (13.9 KB) - added by Mike Hansen 14 years ago.
trac_4135.2.patch (14.2 KB) - added by Timothy Clemans 14 years ago.
trac_4135.3.patch (15.1 KB) - added by Dan Drake 14 years ago.
trac_4135.2.patch rebased for 3.4.1.rc2
trac_4135.4.patch (56.8 KB) - added by Dan Drake 13 years ago.
rebased against 4.1.1 + #6568
trac_4135.5.patch (56.9 KB) - added by Dan Drake 13 years ago.
rebased against mvngu's 4.1.2.alpha0 merge tree

Download all attachments as: .zip

Change History (39)

comment:1 Changed 14 years ago by Timothy Clemans

Depends on #4134 and #2407

comment:2 Changed 14 years ago by Michael Abshoff

Milestone: sage-3.1.2sage-3.1.3

comment:3 Changed 14 years ago by Dan Drake

I'm trying to review this, but I can't get the patches to apply. I'm starting from a clean 3.1.2 tree. Which patches should I apply, and in what order? Should I use a different version to apply patches against?

comment:4 Changed 14 years ago by Timothy Clemans

This ticket has been rebased.

comment:5 Changed 14 years ago by Jason Grout

It seems that the /user page isn't found with an existing notebook. I created a new notebook and the /users page came up.

comment:6 Changed 14 years ago by Timothy Clemans

I've had trouble in the pass with getting administrative users recognized by the Notebook.

comment:7 Changed 14 years ago by Dan Drake

I've applied all the necessary patches to a 3.1.3alpha1 tree, but when I try to start Sage, I get:

/var/tmp/sage-3.1.3.alpha1/local/lib/python2.5/site-packages/sage/server/notebook/twist.py in <module>()
   1935     return False
   1936 
-> 1937 from sage.server.notebook.template import registration_page_template
   1938 from sage.server.notebook.template import login_page_template
   1939 

/var/tmp/sage-3.1.3.alpha1/local/lib/python2.5/site-packages/sage/server/notebook/template.py in <module>()
     38 templates = ['login', 'yes_no', 'failed_login', 'register', 'admin_add_user']
     39 for name in templates:
---> 40     G[name + '_template'] =  PageTemplate(pjoin(path, '%s.template'%name))
     41 
     42 def login_page_template(accounts, default_user, is_username_error=False, is_password_error=False, welcome=None, recover=False):

/var/tmp/sage-3.1.3.alpha1/local/lib/python2.5/site-packages/sage/server/notebook/template.py in __init__(self, filename)
     27 class PageTemplate:
     28     def __init__(self, filename):
---> 29         file = open(filename, 'r')
     30         self.__template = Template(file.read())
     31         file.close()

IOError: [Errno 2] No such file or directory: '/var/tmp/sage-3.1.3.alpha1/data/extcode/notebook/templates/admin_add_user.template'

That's just the last part of the traceback; the rest doesn't seem so interesting...but let me know if you want to see it.

I've looked through the patches from 4134, 2407, and this ticket, and I don't see where the file admin_add_user.template is created. I have all those patches applied (except 4134, which has already been merged); how do I get that file?

comment:8 in reply to:  7 Changed 14 years ago by Timothy Clemans

Replying to ddrake:

I've looked through the patches from 4134, 2407, and this ticket, and I don't see where the file admin_add_user.template is created. I have all those patches applied (except 4134, which has already been merged); how do I get that file?

Did you do hg_extcode.apply('http://trac.sagemath.org/sage_trac/attachment/ticket/4135/extcode-4135_1.patch')

comment:9 Changed 14 years ago by Jason Grout

Or if you're using queues and don't want to mess up your tree, go to $SAGE_ROOT/data/extcode and do:

hg qinit
hg qimport /path/to/extcode-patch
hg qpush

comment:10 Changed 14 years ago by Mike Hansen

Timothy, what is the relationship between this patch and #3923, #3937, and #3949.

comment:11 Changed 14 years ago by Timothy Clemans

Summary: [with patch, needs review] notebook -- user management features[with patch, needs work] notebook -- user management features

comment:12 Changed 14 years ago by Timothy Clemans

Summary: [with patch, needs work] notebook -- user management features[with patch, needs review] notebook -- user management features

Apply just extcode-4135_1.patch and sage-4145_3.patch

comment:13 Changed 14 years ago by Timothy Clemans

Owner: changed from Kelly Boothby to Timothy Clemans
Status: newassigned
Type: defectenhancement

comment:14 Changed 14 years ago by Timothy Clemans

Depends on #3950

comment:15 Changed 14 years ago by Timothy Clemans

Don't apply extcode-4135_1.patch afterall

comment:16 Changed 14 years ago by Timothy Clemans

Apply sage-4135_3.patch and sage-4135_4.patch

Changed 14 years ago by Mike Hansen

Attachment: trac_4135.patch added

comment:17 Changed 14 years ago by Michael Abshoff

Can we get a review for this since this patch is holding up several other patches?

Cheers,

Michael

comment:18 Changed 14 years ago by Dan Drake

I applied mhansen's patch to a clean 3.3.alpha3 tree, deleted my .sage/sage_notebook directory to start fresh, but I still get problems: I log in, click "Manage Users", click "Add user", then put in a name and hit "Create account". The resulting page is blank and has the title "Error | Sage Notebook".

If I hit the back button to get back to the "add a user" page, then click cancel, I get to the user management page, which lists the user I tried to add.

If I click on the user's name, I go to the usual "home page", and it lists the user's name as if I am logged in as that user...but I thought I was logged in as admin? If I then click on Settings, I can go to the user management page and...well, manage users.

Right now, this patch seems to need work. Am I doing something wrong?

comment:19 Changed 14 years ago by Timothy Clemans

Summary: [with patch, needs review] notebook -- user management features[with patch, needs work] notebook -- user management features

Changed 14 years ago by Timothy Clemans

Attachment: trac_4135.2.patch added

comment:20 Changed 14 years ago by Timothy Clemans

Summary: [with patch, needs work] notebook -- user management features[with patch, needs review] notebook -- user management features

There was a bug in the error_message.html template.

comment:21 Changed 14 years ago by Dan Drake

I've applied the patch to a 3.4 tree and all the bits seem to work.

Changed 14 years ago by Dan Drake

Attachment: trac_4135.3.patch added

trac_4135.2.patch rebased for 3.4.1.rc2

comment:22 Changed 14 years ago by Dan Drake

Summary: [with patch, needs review] notebook -- user management features[with patch, positive review] notebook -- user management features

I've rebased this patch for 3.4.1.rc2 and give it a slightly weak positive review -- I still don't know Twisted or the notebook code terribly well, and would appreciate it if someone who knows it better would also take a look at this patch.

comment:23 Changed 14 years ago by Michael Abshoff

Summary: [with patch, positive review] notebook -- user management features[with patch, needs review] notebook -- user management features

Hmm, given this is rather crucial and hard to test code I would be happy if someone else did take another look, so "needs review" to attract someone else.

Cheers,

Michael

comment:24 Changed 14 years ago by Tim Dumol

Summary: [with patch, needs review] notebook -- user management features[with patch, needs work] notebook -- user management features

Applied patch to r12658.

Everything works perfectly except for the cancel link in the Account Settings page. It links to /home, leading to a 404 Error. I believe /home/<username>/ is the proper way to link it.

comment:25 in reply to:  24 Changed 14 years ago by Dan Drake

Replying to timdumol:

Applied patch to r12658.

Everything works perfectly except for the cancel link in the Account Settings page. It links to /home, leading to a 404 Error. I believe /home/<username>/ is the proper way to link it.

I see the same error. I put in a <pre>{{ debug()|e }}</pre> bit (see http://jinja.pocoo.org/1/documentation/designerdoc) into account_settings.html and it looks like the template engine is not getting a username.

comment:26 Changed 13 years ago by wjp

For the cancel button not working, see the patch at #6856. (I created that ticket before seeing the issue was already mentioned here.)

comment:27 Changed 13 years ago by wjp

I don't understand how (or if?) this patch implements the mentioned temporary password functionality. There is a new 'temporary_password' variable, but that doesn't seem to be used.

comment:28 in reply to:  27 Changed 13 years ago by Dan Drake

Replying to wjp:

I don't understand how (or if?) this patch implements the mentioned temporary password functionality. There is a new 'temporary_password' variable, but that doesn't seem to be used.

When one adds a new user, you get a page saying "The temporary password for the new user foo is 52oN5g2a". Unfortunately, that password does not seem to be temporary; the new user can log in and out, and back in again with that password. When I read "temporary password", I think that the user will be able to log in once, and then be forced to change the password right away.

Another problem with the temporary password page is that the title of the page is "Error", even though there's no error.

That said: this patch, along with #6856 (which I just positively reviewed) does give us the basic functionality for adding users and resetting passwords from inside the notebook, which is something I'd really like. The last patch applies cleanly to 4.1.1. What do people think of merging this and opening a new ticket to fix the temporary password bits? (Or changing this patch so that it doesn't claim that the new password is temporary?)

comment:29 Changed 13 years ago by wjp

I'd agree with not holding up this patch because of this, because it is already a very useful feature to have.

This patch does probably have to be rebased against #6568, though.

Changed 13 years ago by Dan Drake

Attachment: trac_4135.4.patch added

rebased against 4.1.1 + #6568

comment:30 Changed 13 years ago by Dan Drake

Authors: Timothy Clemans
Reviewers: Tim Dumol, Dan Drake
Summary: [with patch, needs work] notebook -- user management features[with patch, positive review] notebook -- user management features

Rebased patch up. This should apply cleanly to the release manager's 4.1.2.alpha tree. I am going to change this to a positive review and open a new ticket for the temporary password business.

Release manager: apply only attachment:trac_4135.4.patch.

comment:31 Changed 13 years ago by Dan Drake

The temporary password ticket is #6871.

comment:32 Changed 13 years ago by Minh Van Nguyen

I got two hunk failures when merging trac_4135.4.patch:

[mvngu@mod sage-main]$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4135/trac_4135.4.patch && hg qpush
adding trac_4135.4.patch to series file
applying trac_4135.4.patch
patching file sage/server/notebook/twist.py
Hunk #6 FAILED at 217
Hunk #69 FAILED at 1747
2 out of 102 hunks FAILED -- saving rejects to file sage/server/notebook/twist.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
Errors during apply, please fix and refresh trac_4135.4.patch

This might depend on #6742, #6840, and #6568.

comment:33 in reply to:  32 Changed 13 years ago by Dan Drake

Replying to mvngu:

I got two hunk failures when merging trac_4135.4.patch:

(snip)

This might depend on #6742, #6840, and #6568.

I tried applying the patch here on top of a 4.1.1 tree that already had the patches from those three tickets applied, and I get a different hunk that won't apply. Perhaps I'll wait for the alpha0 tarball to be released and rebase the patch here based on that...or, you could hg serve your merge tree and let me pull from that.

Changed 13 years ago by Dan Drake

Attachment: trac_4135.5.patch added

rebased against mvngu's 4.1.2.alpha0 merge tree

comment:34 Changed 13 years ago by Minh Van Nguyen

Merged in: Sage 4.1.2.alpha0
Resolution: fixed
Status: assignedclosed

Merged trac_4135.5.patch. See #6874 for a follow-up to this ticket.

Note: See TracTickets for help on using tickets.