Project

General

Profile

Actions

Task #2392

closed

Enhance logging and improve security of validation in SSO authentication filter

Added by Petr Fišer almost 4 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Peter Štrunc
Target version:
Start date:
07/15/2020
Due date:
% Done:

100%

Estimated time:
Owner:

Description

The ExtrasSsoIdmAuthenticationFilter.java has issues with validation and logging.
  • In authorize method there is:
    ...
    if (Strings.isNullOrEmpty(token)) {
        return true;
    }
    ..
    

    Which means that if token is null or empty, the authorize method returns true and authorizes the user with god-knows-what privileges. Better have a defensive approach and return false there.
    Fortunately, if the token in empty/null, the authorize should not even get called from the core. So maybe this piece of code is useless here and may be deleted?
  • The class has no useful logging at all. We should fix this to be able to check the flow. It would be handy to have DEBUG logging and to add even some INFO logging, to at least know the filter gets called.
Actions #1

Updated by Marek Klement almost 4 years ago

  • Status changed from New to Needs feedback
  • Assignee set to Marek Klement
  • % Done changed from 0 to 80
  • A Condition above changed as suggested.
  • Added couple of logs
  • Adjusted test to include condition
  • On branch personal/klement/2392-improve-logging-and-security
  • Tests have 78% coverage.
Actions #2

Updated by Petr Fišer almost 4 years ago

Review OK. Just need to add one debug line and test exception throwing on two lines. No additional review necessary I think.

Actions #3

Updated by Marek Klement almost 4 years ago

  • Assignee changed from Marek Klement to Peter Štrunc
  • % Done changed from 80 to 100
  • Changed some logs after review. It should be fine.
Actions #4

Updated by Peter Štrunc over 3 years ago

  • Status changed from Needs feedback to Closed
  • Target version set to 2.5.0

It looks great. Thanks

Actions

Also available in: Atom PDF