simplesamlphp / simplesamlphp-module-oidc

A SimpleSAMLphp module for OIDC OP support.
Other
45 stars 22 forks source link

TypeError: Return value of getClaims() must be of the type array, null returned #193

Closed olifrieda closed 1 year ago

olifrieda commented 1 year ago

If you get this error while trying to login:

TypeError: Return value of SimpleSAML\Module\oidc\Entity\UserEntity::getClaims() must be of the type array, null returned

it might be because of an empty claims column in the database. The reason why this field is empty could be an error while running the json_encode() function. This cannot handle non UTF-8 chars! Especially when you have an LDAP as authsource the attributes from session state may contain non UTF-8 chars. To not run into this issue, non UTF-8 chars should be removed or converted.

https://github.com/simplesamlphp/simplesamlphp-module-oidc/blob/40e2ea3bdd658d409ce52116331419740f5f5233/lib/Entity/UserEntity.php#L66-L89

I have changed these lines:

    public static function fromState(array $state): self
    {
        $user = new self();

        $user->identifier = $state['id'];
        $user->claims = json_decode($state['claims'], true, 512, JSON_THROW_ON_ERROR);
        $user->updatedAt = TimestampGenerator::utc($state['updated_at']);
        $user->createdAt = TimestampGenerator::utc($state['created_at']);

        return $user;
    }

    /**
     * {@inheritdoc}
     */
    public function getState(): array
    {
        return [
            'id' => $this->getIdentifier(),
            'claims' => json_encode($this->utf8ize($this->getClaims()), JSON_THROW_ON_ERROR),
            'updated_at' => $this->getUpdatedAt()->format('Y-m-d H:i:s'),
            'created_at' => $this->getCreatedAt()->format('Y-m-d H:i:s'),
        ];
    }

    private function utf8ize($mixed)
    {
        if (is_array($mixed)) {
            foreach ($mixed as $key => $value) {
                $mixed[$key] = $this->utf8ize($value);
            }
        } elseif (is_string($mixed)) {
            return mb_convert_encoding($mixed, 'UTF-8', 'UTF-8');
        }

        return $mixed;
    }

Maybe someone can fix this issue.

Thanks to https://stackoverflow.com/a/52641198

cicnavi commented 1 year ago

@olifrieda Hi, thanks for bringing this up, it definitely has to be tackled with!

When you use "mb_convert_encoding($mixed, 'UTF-8', 'UTF-8');", don't you just get some replacement chars like question marks for invalid utf8 chars? If that is the case, I would rather just use 'JSON_INVALID_UTF8_IGNORE' or 'JSON_INVALID_UTF8_SUBSTITUTE' flag to simply ignore invalid utf8 chars in the first place.

olifrieda commented 1 year ago

@cicnavi The flags may work too. We should give it a try!