AssertionValidator::validateAudienceRestriction()   A
last analyzed

Complexity

Conditions 4
Paths 4

Size

Total Lines 12

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 6
CRAP Score 4.0466

Importance

Changes 0
Metric Value
dl 0
loc 12
c 0
b 0
f 0
ccs 6
cts 7
cp 0.8571
rs 9.8666
cc 4
nc 4
nop 1
crap 4.0466
1
<?php
2
3
/*
4
 * This file is part of the LightSAML-Core package.
5
 *
6
 * (c) Milos Tomic <[email protected]>
7
 *
8
 * This source file is subject to the MIT license that is bundled
9
 * with this source code in the file LICENSE.
10
 */
11
12
namespace LightSaml\Validator\Model\Assertion;
13
14
use LightSaml\Error\LightSamlValidationException;
15
use LightSaml\Helper;
16
use LightSaml\Model\Assertion\Assertion;
17
use LightSaml\Model\Assertion\AttributeStatement;
18
use LightSaml\Model\Assertion\AudienceRestriction;
19
use LightSaml\Model\Assertion\AuthnStatement;
20
use LightSaml\Model\Assertion\Conditions;
21
use LightSaml\Model\Assertion\OneTimeUse;
22
use LightSaml\Model\Assertion\ProxyRestriction;
23
use LightSaml\SamlConstants;
24
use LightSaml\Validator\Model\NameId\NameIdValidatorInterface;
25
use LightSaml\Validator\Model\Statement\StatementValidatorInterface;
26
use LightSaml\Validator\Model\Subject\SubjectValidatorInterface;
27
28
class AssertionValidator implements AssertionValidatorInterface
29
{
30
    /** @var NameIdValidatorInterface */
31
    protected $nameIdValidator;
32
33
    /** @var SubjectValidatorInterface */
34
    protected $subjectValidator;
35
36
    /** @var StatementValidatorInterface */
37
    protected $statementValidator;
38
39
    /**
40
     * @param NameIdValidatorInterface    $nameIdValidator
41
     * @param SubjectValidatorInterface   $subjectValidator
42
     * @param StatementValidatorInterface $statementValidator
43
     */
44 19
    public function __construct(
45
        NameIdValidatorInterface $nameIdValidator,
46
        SubjectValidatorInterface $subjectValidator,
47
        StatementValidatorInterface $statementValidator
48
    ) {
49 19
        $this->nameIdValidator = $nameIdValidator;
50 19
        $this->subjectValidator = $subjectValidator;
51 19
        $this->statementValidator = $statementValidator;
52 19
    }
53
54
    /**
55
     * @param Assertion $assertion
56
     *
57
     * @return void
58
     */
59 19
    public function validateAssertion(Assertion $assertion)
60
    {
61 19
        $this->validateAssertionAttributes($assertion);
62 13
        $this->validateSubject($assertion);
63 10
        $this->validateConditions($assertion);
64 4
        $this->validateStatements($assertion);
65 4
    }
66
67
    /**
68
     * @param Assertion $assertion
69
     *
70
     * @throws LightSamlValidationException
71
     */
72 19
    protected function validateAssertionAttributes(Assertion $assertion)
73
    {
74 19
        if (false == Helper::validateRequiredString($assertion->getVersion())) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
75 1
            throw new LightSamlValidationException('Assertion element must have the Version attribute set.');
76
        }
77 18
        if (SamlConstants::VERSION_20 != $assertion->getVersion()) {
78 1
            throw new LightSamlValidationException('Assertion element must have the Version attribute value equal to 2.0.');
79
        }
80 17
        if (false == Helper::validateRequiredString($assertion->getId())) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
81 1
            throw new LightSamlValidationException('Assertion element must have the ID attribute set.');
82
        }
83 16
        if (false == Helper::validateIdString($assertion->getId())) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
84 1
            throw new LightSamlValidationException('Assertion element must have an ID attribute with at least 16 characters (the equivalent of 128 bits).');
85
        }
86 15
        if (false == $assertion->getIssueInstantTimestamp()) {
0 ignored issues
show
Bug Best Practice introduced by
It seems like you are loosely comparing $assertion->getIssueInstantTimestamp() of type integer to the boolean false. If you are specifically checking for 0, consider using something more explicit like === 0 instead.
Loading history...
87 1
            throw new LightSamlValidationException('Assertion element must have the IssueInstant attribute set.');
88
        }
89 14
        if (false == $assertion->getIssuer()) {
90 1
            throw new LightSamlValidationException('Assertion element must have an issuer element.');
91
        }
92 13
        $this->nameIdValidator->validateNameId($assertion->getIssuer());
93 13
    }
94
95
    /**
96
     * @param Assertion $assertion
97
     *
98
     * @throws LightSamlValidationException
99
     */
100 13
    protected function validateSubject(Assertion $assertion)
101
    {
102 13
        if (false == $assertion->getSubject()) {
103 3
            if (false == $assertion->getAllItems()) {
104 1
                throw new LightSamlValidationException('Assertion with no Statements must have a subject.');
105
            }
106 2
            foreach ($assertion->getAllItems() as $item) {
107 2
                if ($item instanceof AuthnStatement || $item instanceof AttributeStatement) {
108 2
                    throw new LightSamlValidationException('AuthnStatement, AuthzDecisionStatement and AttributeStatement require a subject.');
109
                }
110
            }
111
        } else {
112 10
            $this->subjectValidator->validateSubject($assertion->getSubject());
0 ignored issues
show
Bug introduced by
It seems like $assertion->getSubject() can be null; however, validateSubject() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
113
        }
114 10
    }
115
116 10
    protected function validateConditions(Assertion $assertion)
117
    {
118 10
        if (false == $assertion->getConditions()) {
119 3
            return;
120
        }
121
122 7
        $this->validateConditionsInterval($assertion->getConditions());
0 ignored issues
show
Bug introduced by
It seems like $assertion->getConditions() can be null; however, validateConditionsInterval() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
123
124 6
        $oneTimeUseSeen = $proxyRestrictionSeen = false;
125
126 6
        foreach ($assertion->getConditions()->getAllItems() as $item) {
127 6
            if ($item instanceof OneTimeUse) {
128 1
                if ($oneTimeUseSeen) {
129 1
                    throw new LightSamlValidationException('Assertion contained more than one condition of type OneTimeUse');
130
                }
131 1
                $oneTimeUseSeen = true;
132 5
            } elseif ($item instanceof ProxyRestriction) {
133 3
                if ($proxyRestrictionSeen) {
134 1
                    throw new LightSamlValidationException('Assertion contained more than one condition of type ProxyRestriction');
135
                }
136 3
                $proxyRestrictionSeen = true;
137
138 3
                $this->validateProxyRestriction($item);
139 2
            } elseif ($item instanceof AudienceRestriction) {
140 4
                $this->validateAudienceRestriction($item);
141
            }
142
        }
143 1
    }
144
145 7
    protected function validateConditionsInterval(Conditions $conditions)
146
    {
147 7
        if ($conditions->getNotBeforeTimestamp() &&
0 ignored issues
show
Bug Best Practice introduced by
The expression $conditions->getNotBeforeTimestamp() of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
Loading history...
148 7
            $conditions->getNotOnOrAfterTimestamp() &&
0 ignored issues
show
Bug Best Practice introduced by
The expression $conditions->getNotOnOrAfterTimestamp() of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
Loading history...
149 7
            $conditions->getNotBeforeTimestamp() > $conditions->getNotOnOrAfterTimestamp()
150
        ) {
151 1
            throw new LightSamlValidationException('Conditions NotBefore MUST BE less than NotOnOrAfter');
152
        }
153 6
    }
154
155
    /**
156
     * @param ProxyRestriction $item
157
     *
158
     * @throws LightSamlValidationException
159
     */
160 3
    protected function validateProxyRestriction(ProxyRestriction $item)
161
    {
162 3
        if (null === $item->getCount() || '' === $item->getCount() || intval($item->getCount()) != $item->getCount() || $item->getCount() < 0) {
0 ignored issues
show
Unused Code Bug introduced by
The strict comparison === seems to always evaluate to false as the types of '' (string) and $item->getCount() (integer) can never be identical. Maybe you want to use a loose comparison == instead?
Loading history...
163 1
            throw new LightSamlValidationException('Count attribute of ProxyRestriction MUST BE a non-negative integer');
164
        }
165
166 2
        if ($item->getAllAudience()) {
167 1
            foreach ($item->getAllAudience() as $audience) {
168 1
                if (false == Helper::validateWellFormedUriString($audience)) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
169 1
                    throw new LightSamlValidationException('ProxyRestriction Audience MUST BE a wellformed uri');
170
                }
171
            }
172
        }
173 1
    }
174
175
    /**
176
     * @param AudienceRestriction $item
177
     *
178
     * @throws LightSamlValidationException
179
     */
180 2
    protected function validateAudienceRestriction(AudienceRestriction $item)
181
    {
182 2
        if (false == $item->getAllAudience()) {
183
            return;
184
        }
185
186 2
        foreach ($item->getAllAudience() as $audience) {
187 2
            if (false == Helper::validateWellFormedUriString($audience)) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
188 2
                throw new LightSamlValidationException('AudienceRestriction MUST BE a wellformed uri');
189
            }
190
        }
191 1
    }
192
193
    /**
194
     * @param Assertion $assertion
195
     */
196 4
    protected function validateStatements(Assertion $assertion)
197
    {
198 4
        if (false == $assertion->getAllItems()) {
199
            return;
200
        }
201
202 4
        foreach ($assertion->getAllItems() as $statement) {
203 4
            $this->statementValidator->validateStatement($statement);
204
        }
205 4
    }
206
}
207