Show server root directory in move dialog

[takluyver]

Attempt at addressing #2204

• I used a table to get them on the same line, but it doesn't fit them to the space neatly. I'm sure there's a better way to lay this out, but I don't know what it is. People who know more CSS than me, feel free to edit the branch directly.
• I think we're using API paths for the editable part, which is going to look weird for Windows users - the fixed part will have backslashes, and the editable part forward slashes.
• Is exposing the path of the server root directory like this at all a concern? I can't see any reason why it would be, but I don't think anything currently exposes it.
• What would this do with non-filesystem-based contents managers?

[Carreau]

What about show the same than in the dashboard with the folder icon ?

[takluyver]

I may be wrong, but I don't think that icon is widely recognised as 'notebook server root directory'.

If we can figure out the points above, I think we should do this for 5.0, and keep thinking about it for the future. I get the impression that our system of only allowing access to the starting folder and subfolders is not well understood. Maybe we should abandon it entirely - though I'm reluctant to do that, as even if it's a poor security measure by itself, it may make it harder to exploit flaws in other security measures - or maybe we should think about ways to make it clearer what's going on.

[ellisonbg]

Nice! A few comments:

• You can get the layout you want using a flexbox hbox ( sorry I can't help code it, swamped with teaching...)
• It is fine to expose the root of the notebook server, just not the full system path, which we do in general hide from the user. I like Matthias's idea of showing the root of the notebook server in the same manner as the file browser itself. I know it isn't standard, but at least consistent.
• Even for non file system content managers, they will still have the notion of paths, so I think they would show the same type of thing as the file browser does.
• I am fine using forward slashes even on windows.

[takluyver]

It is fine to expose the root of the notebook server, just not the full system path

I don't understand the distinction here. Is it OK for the body tag's data attributes to show that the server is running in /home/takluyver? If so, I'm not sure what you mean about the 'full system path'.

[minrk]

@takluyver I think the issue is about the API path, not filesystem path. So rather than /home/takluyver/foo/ [input] it would show /foo/ [input] if the notebook server is running in /home/takluyver and the dialog is at the /tree/foo/ URL.

[minrk]

...which is the notebookPath data already on the page, I believe.

[takluyver]

Right, it already puts the API path of the directory in the input box where you can edit it. The issue (#2204) is that it's not very clear that you need a path where / is the root of the notebook server. This is my attempt to make that clearer.

[ellisonbg]

We shouldn't leak information about the filesystem above the directory where the notebook server was started. So if the notebook server was started in /home/myself and the user was currently in /home/myself/mynotebooks, we would only show /mynotebooks

…

On Wed, Mar 1, 2017 at 7:15 AM, Thomas Kluyver ***@***.***> wrote: Right, it already puts the API path of the directory in the input box where you can edit it. The issue (#2204 <#2204>) is that it's not very clear that you need a path where / is the root of the notebook server. This is my attempt to make that clearer. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2234 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABr0JeTf-9sBSvEW03TdLagAy32jiLiks5rhYuZgaJpZM4MNWV8> .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub bgranger@calpoly.edu and ellisonbg@gmail.com

[Carreau]

Arguably the user is already logged in, so it does not matter much, and mot of the time have access to this info via the kernel.

Re-arguably against myself, the kernel may not be on the same machine, and the user might have a phone user home.

Maybe is root is home show ~/ ?

[blink1073]

On Github, the "root path" is the root of the repo and the path is shown without a leading slash, why not adopt that for this path as well?

[ellisonbg]

I am fine leaving out the leading slash.

…

On Wed, Mar 1, 2017 at 12:13 PM, Steven Silvester ***@***.***> wrote: On Github, the "root path" is the root of the repo and the path is shown without a leading slash, why not adopt that for this path as well? [image: image] <https://cloud.githubusercontent.com/assets/2096628/23479159/2d73080a-fe89-11e6-9970-d1ff567a47bb.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2234 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABr0NR4sg1Zbdc52pcmOiSoyZ8B8Ptuks5rhdFdgaJpZM4MNWV8> .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub bgranger@calpoly.edu and ellisonbg@gmail.com

[takluyver]

1. I'm not clear why the root directory of the server should be secret.

• So long as our other security measures are working, only the authenticated user can see this anyway; we're not broadcasting it to the internet.
• We avoid serving files from outside the server root directory to make it marginally trickier for someone who can bypass those security mechanisms to read e.g. ssh keys. This has always been somewhat doubtful as a security mechanism, and I don't think that it's worth 'protecting' the information of which directory the server is serving like this.
• It's no less protected than the list of files in that directory, which seems more likely to contain sensitive information than the path.
• +1 to compressing the user's home directory to ~ as @Carreau suggested - does that make it more tolerable?

2. We already show the path from the server root directory in the move dialog. We're discussing this because we're concerned that it's not obvious to users how these paths work. I think showing the server root in the dialog too makes it significantly clearer; I don't think the absence of a leading slash makes much difference.

[minrk]

We should make sure to do something sensible for the cases where there is no server_root directory, e.g. when using pgcontents or GDrive. The prefix should probably come from the ContentsManager, not the Application. For example:

1. use ContentsManager.root_display (or a better name)
2. base ContentsManager.root_display = '/'
3. FileContentsManager.root_display = self.root_dir by default
4. allow hiding by overriding .root_display in config

I like showing the file path in the dialog, and +1 to collapsing $HOME to ~.

[minrk]

Another (simpler) option to the above is to use .root_dir always, and not allow FileContentsManagers to override display, only ensure that the base ContentsManager has .root_dir = '/'.

[ellisonbg]

Good points!

…

On Sat, Mar 4, 2017 at 6:10 PM, Min RK ***@***.***> wrote: We should make sure to do something sensible for the cases where there is no server_root directory, e.g. when using pgcontents or GDrive. The prefix should probably come from the ContentsManager, not the Application. For example: 1. use ContentsManager.root_display (or a better name) 2. base ContentsManager.root_display = '/' 3. FileContentsManager.root_display = self.root_dir by default 4. allow hiding by overriding .root_display in config I like showing the file path in the dialog, and +1 to collapsing $HOME to ~. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2234 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABr0IVn1ybuxwabloUOtQkBqRuRtAP-ks5rihmjgaJpZM4MNWV8> .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub bgranger@calpoly.edu and ellisonbg@gmail.com

[takluyver]

I'm away this week, and have very limited time to work on anything, so please feel free to move this forwards without me.

[gnestor]

From what I've gathered from this discussion, we have 2 options:

• The folder icon root:
  
• os.path.basename(os.path.normpath(self.settings['server_root_dir'])) which will get the server root path from ContentsManager like @minrk proposed and will trim the rest of the absolute path:
  

I also updated the styles to use flexbox vs. table like @ellisonbg proposed.

[gnestor]

Another (simpler) option to the above is to use .root_dir always, and not allow FileContentsManagers to override display, only ensure that the base ContentsManager has .root_dir = '/'.

@minrk Let me know if there's a more elegant way to do this.

[minrk]

@gnestor we'll need ContentsManager.root_dir defined (probably just '/') on the base class so that non-file CMs don't fail with AttributeErrors.

[gnestor]

Ok, would that just require adding something like this to https://github.com/jupyter/notebook/blob/master/notebook/services/contents/manager.py#L35:

root_dir = Unicode(config=True)

    @default('root_dir')
    def _default_root_dir(self):
        return '/'

[minrk]

@gnestor yes, but it can be simpler. You only need @default for dynamically-generated defaults. Simple defaults can be declared in the constructor:

root_dir = Unicode('/', config=True)

[minrk]

@gnestor I think the idea was for it to still be an absolute path, but collapse home to ~. So instead of basename, call:

root_dir = self.contents_manager.root_dir
home = os.path.expanduser('~')
if root_dir.startswith(home + os.path.sep):
    # collapse $HOME to ~
    root_dir = '~' + root_dir[len(home):]

This can be done once when defining the server_root_dir above in notebookapp.py, rather than here every time. Here, we can use server_root=self.settings['server_root'], assuming it already has the right form.

[gnestor]

Ok, so should it look like this?

[gnestor]

@minrk Ready for final review

[minrk]

Thanks!