Opened 9 years ago

Closed 9 years ago

#13270 closed defect (fixed)

Restarted notebook server allows user registration

Reported by: novoselt Owned by: jason, mpatel, was
Priority: blocker Milestone: sage-5.2
Component: notebook Keywords:
Cc: kini, jason 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 ppurka)

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 follow-up: Changed 9 years ago by novoselt

Same story on geom.

comment:2 Changed 9 years ago by kcrisman

  • Report Upstream changed from N/A to Not 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 9 years ago by novoselt

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 9 years ago by kcrisman

  • Cc kini jason 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 follow-up: Changed 9 years ago by jason

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

comment:6 in reply to: ↑ 5 Changed 9 years ago by novoselt

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 9 years ago by jdemeyer

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 9 years ago by jason

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 9 years ago by jason

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

comment:10 Changed 9 years ago by jdemeyer

  • Dependencies set to #13121
  • Milestone changed from sage-5.2 to sage-duplicate/invalid/wontfix
  • Priority changed from blocker to major

comment:11 in reply to: ↑ 1 Changed 9 years ago by dimpase

Replying to novoselt:

Same story on geom.

and on OSX 10.6.8

comment:12 follow-up: Changed 9 years ago by dimpase

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 ; follow-up: Changed 9 years ago by 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!

comment:14 in reply to: ↑ 13 ; follow-up: Changed 9 years ago by 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.

comment:15 in reply to: ↑ 14 Changed 9 years ago by dimpase

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 9 years ago by jdemeyer

  • Dependencies changed from #13121 to #11080
  • Milestone changed from sage-duplicate/invalid/wontfix to sage-5.2

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

comment:17 Changed 9 years ago by ppurka

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

comment:18 follow-up: Changed 9 years ago by dimpase

  • Description modified (diff)
  • Status changed from new to needs_review

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

comment:19 Changed 9 years ago by jason

Thanks for taking care of this, guys!

comment:20 in reply to: ↑ 18 ; follow-up: Changed 9 years ago by 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.

comment:21 in reply to: ↑ 20 Changed 9 years ago by dimpase

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 9 years ago by kcrisman

how about just calling it 0.9.0.p0 ?

That seems very logical.

comment:23 Changed 9 years ago by ppurka

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 follow-up: Changed 9 years ago by 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.

comment:25 in reply to: ↑ 24 Changed 9 years ago by ppurka

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 9 years ago by dimpase

  • Description modified (diff)
  • Priority changed from major to blocker

comment:27 Changed 9 years ago by dimpase

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 9 years ago by kcrisman

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 9 years ago by 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 9 years ago by novoselt

Thanks for solving it!!!

comment:31 Changed 9 years ago by ppurka

  • Description modified (diff)
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Fixed upstream, in a later stable release.
  • Status changed from needs_review to positive_review

sagenb-0.9.1 works. Setting to positive review.

comment:32 Changed 9 years ago by jdemeyer

  • Authors set to Jason Grout, Keshav Kini
  • Merged in set to sage-5.2.rc1
  • Resolution set to fixed
  • Reviewers set to Punarbasu Purkayastha
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.