yarn.lock with old deps triggers the github vuln errors, if you update deps
to newest versions grunt commands will stop working. Since we generating
emails here, vuln errors seems needless.
We need to remove grunt dependency anyhow
* Emails: resurrect template notification
* Phantomjs (oh yeah, there is another dev dep phantom :-) was failing for
the generation of the html templates so I had to update the dependencies
in order to fix it. While doing that I update the scripts field and docs
for it as well. yarn.lock is included
* Move splitting of the emails to separate helper function, since more services
coming up that would need to use this functionality
* Add support for enterprise specific email letters. Probably could
be done in the better way, but it's not a priority right now
* Auth: change the error HTTP status codes
* Use 407 HTTP status code for incorrect credentials error
* Improve proxy auth logs
* Remove no longer needed TODO comment
Fixes#18439
It seems `ldap` module introduced new error type of which
multildap module didn't know about.
This broke the multildap login logic
Fixes#18491
Ref #18587
* Auth: Do not search for the user twice
Previously `initContextWithBasicAuth` did not use `LoginUserQuery`, doing
`GetUserByLoginQuery` only i.e. looking user in DB only, things changed when
this function started to check LDAP provider via `LoginUserQuery` (#6940),
however, this request was placed after `GetUserByLoginQuery`, so we first
looking in DB then in the LDAP - if LDAP user hasn't logged in we will
not find it in DB, so `LoginUserQuery` will never be reached.
`LoginUserQuery` request already performs `GetUserByLoginQuery`
request in correct sequence. So we can just remove redundant request.
* Correct sequence execution during authentification &
introduce tests for it
* Move basic auth tests to separate test file, since main test file already
pretty large
* Introduce `testing.go` for the middleware module
* Remove redundant test helper function
* Make handler names more explicit
Ref 5777f65d05Fixes#18329
* Auth: address review comment
* Add tests for errors basic auth cases and moves tests to separate test-case.
Also names test cases consistently
* Add additional test helper
Ref 82661b9f69
* LDAP: nitpicks
* Add more tests
* Correct and clarify comment for Login() method
* Rename methods (hail consistency!)
* Uppercases first letter of the logs everywhere
* Moves method definitions around to more appropriate places
Fixes#18295
* Auth: consistently return same basic auth errors
* Put repeated errors in consts and return only those consts as error strings
* Add tests for errors basic auth cases and moves tests to separate test-case.
Also names test cases consistently
* Add more error logs and makes their messages consistent
* A bit of code style
* Add additional test helper
* Auth: do not expose even incorrect password
* Auth: address review comments
Use `Debug` for the cases when it's an user error
* LDAP: improve POSIX support
* Correctly abtain DN attributes result
* Allow more flexibility with comparison mapping between POSIX group & user
* Add devenv for POSIX LDAP server
* Correct the docs
Fixes#18140
Prompted by the @slim-bean talk (thanks!). With that definition
`GO_FILES` var can be dynamically changed, that will be very helpful
in grafana enterprise repo
* Metrics: remove unused metrics
Metric `M_Grafana_Version` is not used anywhere, nor the mentioned
`M_Grafana_Build_Version`. Seems to be an artefact?
* Metrics: make the naming consistent
* Metrics: add comments to exported vars
* Metrics: use proper naming
Fixes#18110
* Build: allow dynamically change docker image
Right now, only for couple DBs.
With the 2.1, you can do `${TAG:-5.6}` without needing the `.env` file, but
it seems it would be better for back-compat to support `.env` right now.
Fixes#17891
* Build: add prefix for removal process
* Docs: mention makefile command instead of the bash
Since we are moving to makefile for things and make command
simplifies some of the actions you have to perform to work with
different environments
* Docs: mention dynamic tags in the docs
* Docs: avoid name collision
Fixes#16107
* Add support for `is_disabled` to `CreateUser()`
* Add support for `is_disabled` to `SearchUsers()`
Had to add it as a `string` type not as `bool`, since if that's property
is omitted, we would have add it to SQL request, which might be dangerous
* Restructure desctructive tests and add more
Before this applied, go binary is referenced inconsistently in makefile.
Sometimes with go modules enabled and sometimes not, sometimes through
a variable and sometimes not.
This change makes all the references in makefile consistent
* LDAP:Docs: `active_sync_enabled` setting
Mention `active_sync_enabled` setting and enable it by default
* LDAP: move "disableExternalUser" method
Idea behind new design of the LDAP module is to minimise conflation
between other parts of the system, so it would decoupled as much as
possible from stuff like database, HTTP transport and etc.
Following "Do One Thing and Do It Well" Unix philosophy principal, other things
could be better fitted on the consumer side of things.
Which what this commit trying to archive
* LDAP: correct user/admin binding
The second binding was not happening, so if the admin login/password
in LDAP configuration was correct, anyone could had login as anyone using
incorrect password
* LDAP: Divide the requests
Active Directory does indeed have a limitation with 1000 results
per search (default of course).
However, that limitation can be workaround with the pagination search feature,
meaning `pagination` number is how many times LDAP compatible server will be
requested by the client with specified amount of users (like 1000). That feature
already embeded with LDAP compatible client (including our `go-ldap`).
But slapd server has by default stricter settings. First, limitation is not 1000
but 500, second, pagination workaround presumably (information about it a bit
scarce and I still not sure on some of the details from my own testing)
cannot be workaround with pagination feature.
See
https://www.openldap.org/doc/admin24/limits.htmlhttps://serverfault.com/questions/328671/paging-using-ldapsearchhashicorp/vault#4162 - not sure why they were hitting the limit in
the first place, since `go-ldap` doesn't have one by default.
But, given all that, for me `ldapsearch` command with same request
as with `go-ldap` still returns more then 500 results, it can even return
as much as 10500 items (probably more).
So either there is some differences with implementation of the LDAP search
between `go-ldap` module and `ldapsearch` or I am missing a step :/.
In the wild (see serverfault link), apparently, people still hitting that
limitation even with `ldapsearch`, so it still seems to be an issue.
But, nevertheless, I'm still confused by this incoherence.
To workaround it, I divide the request by no more then
500 items per search
* Build: use golangci-lint as a make command
* Since gometalinter was deprecated in favor of golangci-lint so it was
replaced by it. Responsibilities held by the gometalinter was moved to
golangci-lint
* There was some changes in implementation (that was also mentioned in
the code comment) between the tools, which uncovered couple errors
in the code. Those issues were either solved or disabled by
the inline comments
* Introduce the golangci-lint config, to make their
configuration more manageable
* Build: replace backend-lint.sh script with make
* Add LDAP config instead sed use
* Add container name
* Add SizeLimit option to client and to server.
Probably useless at this point, but it's better to have it then otherwise
* LDAP: use only one struct
* Use only models.ExternalUserInfo
* Add additional helper method :/
* Move all the helpers to one module
* LDAP: refactoring
* Rename some of the public methods and change their behaviour
* Remove outdated methods
* Simplify logic
* More tests
There is no and never were tests for settings.go, added tests for helper
methods (cover is now about 100% for them). Added tests for the main
LDAP logic, but there is some stuff to add. Dial() is not tested and not
decoupled. It might be a challenge to do it properly
* Restructure tests:
* they wouldn't depend on external modules
* more consistent naming
* logical division
* More guards for erroneous paths
* Login: make login service an explicit dependency
* LDAP: remove no longer needed test helper fns
* LDAP: remove useless import
* LDAP: Use new interface in multildap module
* LDAP: corrections for the groups of multiple users
* In case there is several users their groups weren't detected correctly
* Simplify helpers module
Same idea as with the `gosec`, `revive` and etc.
Allows you to execute `make bra` as pinned local dependency instead
as global one. And therefore alleviates dependency installation issues.
README also updated - I was thinking to provide verbose installation instructions
for it. Since with that way we assume make is installed on the
developer system.
But it might be premature right now?
Otherwise changed data will not get refreshed to the previous
state.
Which I think is not evident (unless you are familar with docker-compose).
and therefore error-prone (at least it did for me)
With the previous configuration `docker-compose build` was always failing.
This moves the dockerfiles in the parent dir and changes paths as a result.
Ref moby/moby#2745
* Removes Add/Remove methods
* Publicise necessary fields and methods so we could extend it
* Publicise mock API
* More comments and additional simplifications
* Sync with master
Still having low coverage :/ - should be addressed in #17208
* incapsulates multipleldap logic under one module
* abstracts users upsert and get logic
* changes some of the text error messages and import sort sequence
* heavily refactors the LDAP module – LDAP module now only deals with LDAP related behaviour
* integrates affected auth_proxy module and their tests
* refactoring of the auth_proxy logic
* Chore: explore possibilities of using makefile
This is an exploratory commit - I wanted to see how
revive/gosec linters could be integrated with makefile and our build scripts.
Looks better then I expected :)
* Chore: make revive happy
Revive execution was not supplied with path, if you restore there is couple
errors that were popping up - so I fixed them
* Chore: make revive happy
* Feature: provide multildap server configuration
* Add multildap server configuration for devenv
* Change some of the notes for openldap devenv configuration
* Change the maintainer label for *main* dockerfile and of the devenv one
Re-checked the multildap logic - everything seems to be working fine
* Feature: introduce LdapActiveSyncEnabled setting
We probably remove it after the active sync is done.
But at the moment we do not want to affect the current users
with not fully tested feature
* Chore: move settings in more logical order
* Feature: add cron setting for the ldap settings
* Move ldap configuration read to special function
* Introduce cron setting (no docs for it yet, pending approval)
* Chore: duplicate ldap module as a service
* Feature: implement active sync
This is very early preliminary implementation of active sync.
There is only one thing that's going right for this code - it works.
Aside from that, there is no tests, error handling, docs, transactions,
it's very much duplicative and etc.
But this is the overall direction with architecture I'm going for
* Chore: introduce login service
* Chore: gradually switch to ldap service
* Chore: use new approach for auth_proxy
* Chore: use new approach along with refactoring
* Chore: use new ldap interface for auth_proxy
* Chore: improve auth_proxy and subsequently ldap
* Chore: more of the refactoring bits
* Chore: address comments from code review
* Chore: more refactoring stuff
* Chore: make linter happy
* Chore: add cron dep for grafana enterprise
* Chore: initialize config package var
* Chore: disable gosec for now
* Chore: update dependencies
* Chore: remove unused module
* Chore: address review comments
* Chore: make linter happy
* Feature: add cron setting for the ldap settings
* Move ldap configuration read to special function
* Introduce cron setting (no docs for it yet, pending approval)
* Chore: address code review comments
* Chore: remove session storage references
* Small refactoring of the settings module
* Update docs - remove references for the session storage
* Update config files (sample and default configs)
* Add tests for warning during the config load on defined storage cache
* Remove all references to session storage
* Remove macaron session dependency
* Remove leftovers
* Fix: address review comments
* Fix: remove old deps
* Fix: add skipStaticRootValidation = true to tests
* Fix: improve the docs and warning message
As per discussion in here - https://github.com/grafana/grafana/pull/16445/files#r273026255
* Chore: make linter happy
Fixes#16148
Ref #16114
* Chore: refactor auth proxy
Introduced the helper struct for auth_proxy middleware.
Added couple unit-tests, but it seems "integration" tests already cover
most of the code paths.
Although it might be good idea to test every bit of it, hm.
Haven't refactored the extraction of the header logic that much
Fixes#16147
* Fix: make linters happy
This is basically implementation of the https://github.com/grafana/grafana/issues/8900#issuecomment-435437167
points, except for the type conversion bit.
I tried to implement idea mentioned in cockroachdb ticket (see below).
And it is possible, but it complicates things as lot - not only we have to
have 4 SQL statements instead of one, but we would have to copy the column
structure as well - PK, FG, indexes and stuff, plus there will
be additional downtime with this approach.
So idea for this pull is to prepare our SQL as much as possible, so when
cockroachdb will add support for full type conversions, we could easilly add
support for it as well.
* Add `CASCADE` to `DROP INDEX` statement
* Make string conversions explicit
Thanks @Luit
Ref #8900
Ref cockroach/cockroach#9851
See https://github.com/securego/gosec for more info.
Disabled a lot of rules. I guess we should go through them
and recheck if we really need to disable some
Fixes#16204
Since we do not like some of the default golint rules,
this commit proposes to use https://github.com/mgechev/revive.
And potential revive speed-up should't hurt :).
Right now, presented config (./conf/revive.toml) is permissive,
we might improve it over time however. Fixes for found revive
issues in the code are very limited so it wouldn't be large to review.
Also in this commit:
* Add annotations for makefile commands and declare phony targets
* Rename "gometalinter" script and CI command to "lint"
since we are doing there a bit more then using gometalinter package
* Add Makefile rules to .editorconfig
* Documentation which mentioned "golint" replaced with revive
Fixes#16109
Ref #16160
Oleg Gaidarenko