Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

devfile-registry: python-django app cannot preview #19459

Closed
1 of 23 tasks
sunix opened this issue Mar 30, 2021 · 6 comments
Closed
1 of 23 tasks

devfile-registry: python-django app cannot preview #19459

sunix opened this issue Mar 30, 2021 · 6 comments
Assignees
Labels
area/devfile-registry good first issue Community, this issue looks easy to start with for a new contributor. Just take it. We'll help you! kind/bug Outline of a bug - must adhere to the bug report template. severity/P2 Has a minor but important impact to the usage or development of the system. sprint/current
Milestone

Comments

@sunix
Copy link
Contributor

sunix commented Mar 30, 2021

Describe the bug

Python Django devfile cannot preview the application.

Che version

  • latest
  • nightly
  • other: please specify

Steps to reproduce

  1. Start the Django Python devfile from the Get Started Page
  2. Run these commands:
    • set up venv
    • install dependencies
    • migrate
    • run server
  3. Che propose to open the preview
  4. the preview fails to connect
  5. Opening in a new tab works fine

Expected behavior

It should show the right page in the preview panel

Could be fixed by setting X_FRAME_OPTIONS to allowall.

  - name: run server
    actions:
      - workdir: '${CHE_PROJECTS_ROOT}/django-realworld-example-app'
        type: exec
        command: |
          . ${CHE_PROJECTS_ROOT}/.venv/bin/activate && export DEBUG_MODE=False && cp conduit/settings.py local_settings.py && echo "X_FRAME_OPTIONS = 'ALLOWALL'" >> local_settings.py && python manage.py runserver --settings local_settings 0.0.0.0:7000
        component: python
  - name: run server in debug mode
    actions:
      - workdir: '${CHE_PROJECTS_ROOT}/django-realworld-example-app'
        type: exec
        command: |
          . ${CHE_PROJECTS_ROOT}/.venv/bin/activate &&  export DEBUG_MODE=True && && cp conduit/settings.py local_settings.py && echo "X_FRAME_OPTIONS = 'ALLOWALL'" >> local_settings.py && python manage.py runserver --settings local_settings 0.0.0.0:7000 --noreload --nothreading
        component: python

Runtime

  • kubernetes (include output of kubectl version)
  • Openshift (include output of oc version)
  • minikube (include output of minikube version and kubectl version)
  • minishift (include output of minishift version and oc version)
  • docker-desktop + K8S (include output of docker version and kubectl version)
  • other: (please specify)

Screenshots

Eclipse Che_302

Installation method

  • chectl
    • provide a full command that was used to deploy Eclipse Che (including the output)
    • provide an output of chectl version command
  • OperatorHub
  • I don't know

Environment

  • my computer
    • Windows
    • Linux
    • macOS
  • Cloud
    • Amazon
    • Azure
    • GCE
    • other (please specify)
  • Dev Sandbox (workspaces.openshift.com)
  • other: please specify

Eclipse Che Logs

Additional context

@sunix sunix added the kind/bug Outline of a bug - must adhere to the bug report template. label Mar 30, 2021
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Mar 30, 2021
@sunix sunix added the good first issue Community, this issue looks easy to start with for a new contributor. Just take it. We'll help you! label Mar 30, 2021
@mshaposhnik mshaposhnik added area/devfile-registry severity/P2 Has a minor but important impact to the usage or development of the system. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Mar 30, 2021
@ericwill ericwill mentioned this issue Apr 8, 2021
42 tasks
@ericwill ericwill added this to the 7.30 milestone Apr 14, 2021
@ericwill ericwill modified the milestones: 7.30, 7.31 Apr 27, 2021
@ericwill ericwill mentioned this issue Apr 29, 2021
32 tasks
@vitaliy-guliy
Copy link
Contributor

Django replies with x-frame-options: DENY HTTP header.
It block opening a page inside iframe.

I disabled django.middleware.clickjacking.XFrameOptionsMiddleware module, that adds the header and the sample become working.

Screenshot from 2021-05-12 15-00-58

There is a PR che-samples/django-realworld-example-app#3 removing the middleware from Django sample.

@tsmaeder
Copy link
Contributor

Since @vitaliy-guliy involved me, I'll chime in: IMO, the question is whether blocking usage in a frame is good security practice or not: we don't want to give a bad example. If that is so, the fact that it doesn't work in the preview is a feature, not a bug. If it's not security relevant, by all means let's allow-all,

@vitaliy-guliy
Copy link
Contributor

Since @vitaliy-guliy involved me, I'll chime in: IMO, the question is whether blocking usage in a frame is good security practice or not: we don't want to give a bad example. If that is so, the fact that it doesn't work in the preview is a feature, not a bug. If it's not security relevant, by all means let's allow-all,

The doc says that only DENY and SAMEORIGIN allowed for the header https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options

With SAMEORIGIN it will not work since Che-Theia and python http server are running inside other container have different domains.

There is also a simple doc describing the logic of the module https://docs.djangoproject.com/en/1.11/_modules/django/middleware/clickjacking/

Trying with allow-all, just to test..

@vitaliy-guliy
Copy link
Contributor

vitaliy-guliy commented May 12, 2021

@tsmaeder it works fine with allow-all.
This is none-standard value (https://en.wikipedia.org/wiki/Clickjacking#X-Frame-Options), browser just ignores it.

The PR che-samples/django-realworld-example-app#3 is updated

@ericwill
Copy link
Contributor

@sunix
Copy link
Contributor Author

sunix commented May 13, 2021

I would not make any modifications on the original sample: the fork is just to prevent changes that may break our devfiles. the more we fork, the harder it would be to maintain. I would rather set xframe-options in the Che command as suggested in the description.
If the application is not supposed to be running in a frame, we shouldn't change it. Imagine this being a real app: the frame should be allowed only in dev mode (in Che) not in production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-registry good first issue Community, this issue looks easy to start with for a new contributor. Just take it. We'll help you! kind/bug Outline of a bug - must adhere to the bug report template. severity/P2 Has a minor but important impact to the usage or development of the system. sprint/current
Projects
None yet
Development

No branches or pull requests

6 participants