search

morincer / teamcity-plugin-saml

The plug-in adds ability to authenticate users by SAML-based SSO providers (like Okta, Onelogin etc.)
MIT License
24 stars 16 forks source link

SamlCsrfCheck.isSafe causes load of the SAML plugin settings from disk on every request #116

Open pavelsher opened 1 year ago

pavelsher commented 1 year ago

Every request which is going through SamlCsrfCheck.isSafe causes loading of the plugin settings from disk. TeamCity calls CsrfCheck extensions on every request be it GET, or anything else. It seems like SamlCsrfCheck could at lease check that the request is not GET and only in this case perform the load of the settings. A better approach would be cache the plugin settings in memory and reload them more rarely.

at [java.base@11.0.16.1](mailto:java.base@11.0.16.1)/java.io.FileInputStream.open0(Native Method)
at [java.base@11.0.16.1](mailto:java.base@11.0.16.1)/java.io.FileInputStream.open(FileInputStream.java:219)
at [java.base@11.0.16.1](mailto:java.base@11.0.16.1)/java.io.FileInputStream.<init>(FileInputStream.java:157)
at com.fasterxml.jackson.core.JsonFactory.createParser(JsonFactory.java:1029)
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3494)
at jetbrains.buildServer.auth.saml.plugin.SamlPluginSettingsStorageImpl.load(SamlPluginSettingsStorageImpl.java:34)
at jetbrains.buildServer.web.SamlCsrfCheck.isSafe(SamlCsrfCheck.java:32)
at jetbrains.buildServer.web.CSRFFilter.validateRequest(CSRFFilter.java:119)
at jetbrains.buildServer.controllers.interceptors.AuthorizationInterceptorImpl.checkCsrfWithAuthenticatedUser(AuthorizationInterceptorImpl.java:30)
morincer commented 1 year ago

Hi Pavel, thanks for the report. Will implement in upcoming release.

Linfar commented 5 months ago

I've opened a pull request to add caching to settings: https://github.com/morincer/teamcity-plugin-saml/pull/124