Opened 10 years ago

Closed 10 years ago

#13270 closed defect (fixed)

Restarted notebook server allows user registration

Reported by: Andrey Novoseltsev Owned by: jason, mpatel, was
Priority: blocker Milestone: sage-5.2
Component: notebook Keywords:
Cc: Keshav Kini, Jason Grout Merged in: sage-5.2.rc1
Authors: Jason Grout, Keshav Kini Reviewers: Punarbasu Purkayastha
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: Commit:
Dependencies: #11080 Stopgaps:

Status badges

Description (last modified by Punarbasu Purkayastha)

I observe this on two Linux Mint machines: one runs 11th version, another Debian Edition.

  1. Compile sage-5.2.rc0.
  2. Start it.
  3. Start notebook either without parameters or to start remotely accessible secure server.
  4. User registration is enabled (it was NOT before).
  5. Turn it off in the settings and save.
  6. It is indeed off.
  7. Stop the server and start it again.
  8. Use registration is enabled.

this is fixed in updated spkg Corresponding upstream release: sagenb-0.9.1

Change History (32)

comment:1 Changed 10 years ago by Andrey Novoseltsev

Same story on geom.

comment:2 Changed 10 years ago by Karl-Dieter Crisman

Report Upstream: N/ANot yet reported upstream; Will do shortly.

You should probably report this as an issue on Github as well, and refer back here.

comment:3 Changed 10 years ago by Andrey Novoseltsev

I think that the current Github version is working - when I installed #13121, I have noticed the issue but was not able to reproduce it. My guess is that the registration was enabled by Sage own version, then I have noticed it and turned it off with #13121 and everything was fine until I tried clean sage-5.2.rc0 without #13121. Anyway, it does not seem that anyone else is concerned that the next version of Sage will quietly open your machine to anyone, if you try to use the notebook remotely...

comment:4 Changed 10 years ago by Karl-Dieter Crisman

Cc: Keshav Kini Jason Grout added

Anyway, it does not seem that anyone else is concerned that the next version of Sage will quietly open your machine to anyone, if you try to use the notebook remotely...

I wouldn't be so sure. But maybe no one has an obvious fix. You might want to email someone directly to ask where this might have changed in Github. It does seems like a reasonable blocker, for the security issue.

comment:5 Changed 10 years ago by Jason Grout

This is the first I've heard of this. So do you say that #13121 fixes this?

comment:6 in reply to:  5 Changed 10 years ago by Andrey Novoseltsev

Replying to jason:

This is the first I've heard of this. So do you say that #13121 fixes this?

I *think* so. I've originally posted about it there as a one time issue since I couldn't reproduce it. It came back for me in rc0 (reported in the first two replies on sage-release). Switching between clean rc0 or beta1+13121 I can turn this bug on and off, so my guess is that it got fixed in the notebook along the way, but rc0 has too old of a version.

comment:7 Changed 10 years ago by Jeroen Demeyer

So what's the plan then? Postpone the release of sage-5.2 and include #13121 or release sage-5.2 anyway and fix it in sage-5.3?

Considering this is a blocker, probably you should create a thread on sage-notebook asking for opinions.

comment:8 Changed 10 years ago by Jason Grout

With what appears to be a pretty bad security issue, I think delaying 5.2 and considering #13121 a blocker might be best. Keshav: you're the current sagenb guru---what do you say?

comment:9 Changed 10 years ago by Jason Grout

(that said, I haven't had time to look at this; I've just been following the comments here).

comment:10 Changed 10 years ago by Jeroen Demeyer

Dependencies: #13121
Milestone: sage-5.2sage-duplicate/invalid/wontfix
Priority: blockermajor

comment:11 in reply to:  1 Changed 10 years ago by Dima Pasechnik

Replying to novoselt:

Same story on geom.

and on OSX 10.6.8

comment:12 Changed 10 years ago by Dima Pasechnik

I think it is some semi-trivial logic bug related to using None and False. If in sagenb/notebook/run_notebook.py I change the line 224 from

             accounts      = None,

to

             accounts      = False,

then the setting for Enable user registration stays off, instead of staying on. That is, when I start the notebook server, it is always off rather than on, and saving this option has no effect. As opposed to the scenario described on the ticket, when it always stays on.

comment:13 in reply to:  12 ; Changed 10 years ago by Dima Pasechnik

Replying to dimpase:

I think it is some semi-trivial logic bug related to using None and False.

it looks as if the patch just proposed on #11080 fixes this!

comment:14 in reply to:  13 ; Changed 10 years ago by Dima Pasechnik

Replying to dimpase:

Replying to dimpase:

I think it is some semi-trivial logic bug related to using None and False.

it looks as if the patch just proposed on #11080 fixes this!

unfortunately, not quite so. See #11080 for more.

comment:15 in reply to:  14 Changed 10 years ago by Dima Pasechnik

Replying to dimpase:

Replying to dimpase:

Replying to dimpase:

I think it is some semi-trivial logic bug related to using None and False.

it looks as if the patch just proposed on #11080 fixes this!

unfortunately, not quite so. See #11080 for more.

The patch works, I just was confused about the functionality under different scenarios! Let's beg Jeroen to re-merge these patches.

comment:16 Changed 10 years ago by Jeroen Demeyer

Dependencies: #13121#11080
Milestone: sage-duplicate/invalid/wontfixsage-5.2

A new sagenb spkg needs to be created with the fix.

comment:17 Changed 10 years ago by Punarbasu Purkayastha

Sorry, I didn't see that the status of this ticket was changed. Please see comment:344 of #11080.

comment:18 Changed 10 years ago by Dima Pasechnik

Description: modified (diff)
Status: newneeds_review

I applied the patch the old-fashioned way and uploaded the resulting spkg - cf. the link in the description.

comment:19 Changed 10 years ago by Jason Grout

Thanks for taking care of this, guys!

comment:20 in reply to:  18 ; Changed 10 years ago by Punarbasu Purkayastha

Replying to dimpase:

I applied the patch the old-fashioned way and uploaded the resulting spkg - cf. the link in the description.

The spkg works. But it needs the versioning changed everywhere from 0.9.0 to 0.9.1. The install log shows lots of 0.9.0.

comment:21 in reply to:  20 Changed 10 years ago by Dima Pasechnik

Replying to ppurka:

Replying to dimpase:

I applied the patch the old-fashioned way and uploaded the resulting spkg - cf. the link in the description.

The spkg works. But it needs the versioning changed everywhere from 0.9.0 to 0.9.1. The install log shows lots of 0.9.0.

how about just calling it 0.9.0.p0 ?

comment:22 Changed 10 years ago by Karl-Dieter Crisman

how about just calling it 0.9.0.p0 ?

That seems very logical.

comment:23 Changed 10 years ago by Punarbasu Purkayastha

Well, anything other than 0.9.0 is fine as long as it shows that it is different from 0.9.0 since the upstream has a tag of 0.9.0 that points to a particular state of the sagenb.

What is the rationale behind the .p? suffix? Is it used only for bugfixes?

comment:24 Changed 10 years ago by Jason Grout

.p* stands for "patch level" or "patch". It's a way of noting patches to upstream releases, and seems entirely appropriate here.

That way we don't have to backport the patch to 0.9.0 in the git tree. Just apply the patch here "downstream". It is already merged in the next release upstream.

comment:25 in reply to:  24 Changed 10 years ago by Punarbasu Purkayastha

Replying to jason:

.p* stands for "patch level" or "patch". It's a way of noting patches to upstream releases, and seems entirely appropriate here.

That way we don't have to backport the patch to 0.9.0 in the git tree. Just apply the patch here "downstream". It is already merged in the next release upstream.

Great! Then it is just fine to rename the spkg to 0.9.0.p0.

comment:26 Changed 10 years ago by Dima Pasechnik

Description: modified (diff)
Priority: majorblocker

comment:27 Changed 10 years ago by Dima Pasechnik

I have corrected the name to make it sagenb-0.9.0.p0.spkg.

Strictly speaking, one should have put the patch into the root subdirectory of spkg and fix spkg-install to apply it to the pristine 0.9.0. Instead I just patched src/sagenb subdirectory of spkg. As this is a one-time event, and in the future the releases of sagenb will be git-based, I hope it's OK.

comment:28 Changed 10 years ago by Karl-Dieter Crisman

Sure hope so. Having the infrastructure for the patch in here would be redundant, and also sagenb is not really as upstream as one might think... maybe it's more like halfway up one of those trout ladders one sees in the Pacific Northwest.

comment:29 Changed 10 years ago by Keshav Kini

Hi everyone, sorry for the delay in responding.

I cherry picked the commit Jason identified onto 0.9.0 and released a sagenb 0.9.1. SPKG is here: http://wstein.org/home/keshav/files/sagenb-0.9.1.spkg .

Thanks for finding the problem :)

comment:30 Changed 10 years ago by Andrey Novoseltsev

Thanks for solving it!!!

comment:31 Changed 10 years ago by Punarbasu Purkayastha

Description: modified (diff)
Report Upstream: Not yet reported upstream; Will do shortly.Fixed upstream, in a later stable release.
Status: needs_reviewpositive_review

sagenb-0.9.1 works. Setting to positive review.

comment:32 Changed 10 years ago by Jeroen Demeyer

Authors: Jason Grout, Keshav Kini
Merged in: sage-5.2.rc1
Resolution: fixed
Reviewers: Punarbasu Purkayastha
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.