Moodle

Missing standard access control in /portfolio/*.php

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Portfolio API
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

The files /portfolio/*.php do not have require_login()
Those scripts are using $USER and $SESSION, adding require logins and adding forgotten kill switch into portfolio/file.php

Please review and test

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Petr Škoda added a comment -

silly question, why is add.php using optional_param() all over the place?

It would be also nice if each script contained a short description describing what it does, especially when is uses something from session created by other scripts.

Show
Petr Škoda added a comment - silly question, why is add.php using optional_param() all over the place? It would be also nice if each script contained a short description describing what it does, especially when is uses something from session created by other scripts.
Hide
Permalink
Penny Leach added a comment -

Hey Petr, good catch. I'll add docs too.

What do you want me to review? There's no patch on this bug and nothing in the version control tab?

I don't understand your comment about optional_param - that isn't deprecated, what's the problem?

Show
Penny Leach added a comment - Hey Petr, good catch. I'll add docs too. What do you want me to review? There's no patch on this bug and nothing in the version control tab? I don't understand your comment about optional_param - that isn't deprecated, what's the problem?
Hide
Permalink
Petr Škoda added a comment -

ooops, I must have used some other bug number
see cvs

Show
Petr Škoda added a comment - ooops, I must have used some other bug number see cvs
Hide
Permalink
Penny Leach added a comment -

looks fine.I'll look at the docs as time allows.

Petr -still waiting on hearing back on the problem with optional_param.

Show
Penny Leach added a comment - looks fine.I'll look at the docs as time allows. Petr -still waiting on hearing back on the problem with optional_param.
Hide
Permalink
Petr Škoda added a comment -

from http://docs.moodle.org/en/Development:Coding:

8. Wherever possible group all your required_param(), optional_param() and other variables initialisation at the beginning of each file to make them easy to find.

Show
Petr Škoda added a comment - from http://docs.moodle.org/en/Development:Coding: 8. Wherever possible group all your required_param(), optional_param() and other variables initialisation at the beginning of each file to make them easy to find.
Hide
Permalink
Penny Leach added a comment -

fair enough.

Show
Penny Leach added a comment - fair enough.
Hide
Permalink
Petr Škoda added a comment -

I suppose in this case could be separate groups at the start of each case/if

Show
Petr Škoda added a comment - I suppose in this case could be separate groups at the start of each case/if
Hide
Permalink
Penny Leach added a comment -

I think I was probably trying to avoid unnecessary variable assignment, but it really doesn't matter. I will just put it all at the top.

Show
Penny Leach added a comment - I think I was probably trying to avoid unnecessary variable assignment, but it really doesn't matter. I will just put it all at the top.
Hide
Permalink
Penny Leach added a comment -

added docs and moved all optional_param and required_param to the top.

Show
Penny Leach added a comment - added docs and moved all optional_param and required_param to the top.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: